fix: tagging secrets not being scoped by sender (#24772)

This fixes a scoping bug where an account's ivsk would be used to
produce an ECDH shared secret even if that account was not in scope. The
base wallet class was adjusted to put these in scope when an explicit
sender is selected for tagged messages.

Author: nventuro

yarn-project/cli-wallet/src/utils/wallet.ts

@@ -145,7 +145,10 @@ export class CLIWallet extends BaseWallet {
     increasedFee: InteractionFeeOptions,
   ): Promise<TxProvingResult> {
     const cancellationTxRequest = await this.createCancellationTxExecutionRequest(from, txNonce, increasedFee);
-    return await this.pxe.proveTx(cancellationTxRequest, { scopes: this.scopesFrom(from), senderForTags: from });
+    return await this.pxe.proveTx(cancellationTxRequest, {
+      scopes: this.scopesFrom(from, [], undefined),
+      senderForTags: from,
+    });
   }
 
   override async getAccountFromAddress(address: AztecAddress) {
@@ -311,7 +314,7 @@ export class CLIWallet extends BaseWallet {
     opts: SimulateViaEntrypointOptions,
   ): Promise<TxSimulationResultWithAppOffset> {
     const { from, feeOptions, additionalScopes, sendMessagesAs } = opts;
-    const scopes = this.scopesFrom(from, additionalScopes);
+    const scopes = this.scopesFrom(from, additionalScopes ?? [], sendMessagesAs);
     const feeExecutionPayload = await feeOptions.walletFeePaymentMethod?.getExecutionPayload();
     const finalExecutionPayload = feeExecutionPayload
       ? mergeExecutionPayloads([feeExecutionPayload, executionPayload])

yarn-project/end-to-end/src/test-wallet/test_wallet.ts

@@ -318,7 +318,7 @@ export class TestWallet extends BaseWallet {
     opts: SimulateViaEntrypointOptions,
   ): Promise<TxSimulationResultWithAppOffset> {
     const { from, feeOptions, additionalScopes, skipTxValidation, skipFeeEnforcement, sendMessagesAs } = opts;
-    const scopes = this.scopesFrom(from, additionalScopes);
+    const scopes = this.scopesFrom(from, additionalScopes ?? [], sendMessagesAs);
     const skipKernels = this.simulationMode !== 'full';
     const useOverride = this.simulationMode === 'kernelless-override';
 
@@ -384,7 +384,7 @@ export class TestWallet extends BaseWallet {
     });
     const txRequest = await this.createTxExecutionRequestFromPayloadAndFee(exec, opts.from, fee);
     const txProvingResult = await this.pxe.proveTx(txRequest, {
-      scopes: this.scopesFrom(opts.from, opts.additionalScopes),
+      scopes: this.scopesFrom(opts.from, opts.additionalScopes ?? [], opts.sendMessagesAs),
       senderForTags: this.senderForTagsFrom(opts.from, opts.sendMessagesAs),
     });
     return new ProvenTx(

yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.test.ts

@@ -1,12 +1,13 @@
 import { MAX_PROCESSABLE_L2_GAS, MAX_TX_DA_GAS } from '@aztec/constants';
 import { Fr } from '@aztec/foundation/curves/bn254';
-import { Point } from '@aztec/foundation/curves/grumpkin';
+import { GrumpkinScalar, Point } from '@aztec/foundation/curves/grumpkin';
 import type { KeyStore } from '@aztec/key-store';
 import { WASMSimulator } from '@aztec/simulator/client';
 import { FunctionSelector } from '@aztec/stdlib/abi';
 import type { AuthWitness } from '@aztec/stdlib/auth-witness';
 import { AztecAddress } from '@aztec/stdlib/aztec-address';
 import type { L2TipsProvider } from '@aztec/stdlib/block';
+import { CompleteAddress } from '@aztec/stdlib/contract';
 import { Gas, GasFees, GasSettings } from '@aztec/stdlib/gas';
 import type { AztecNode } from '@aztec/stdlib/interfaces/server';
 import { AppTaggingSecret, AppTaggingSecretKind } from '@aztec/stdlib/logs';
@@ -80,6 +81,35 @@ describe('PrivateExecutionOracle', () => {
     });
   });
 
+  describe('getAppTaggingSecret', () => {
+    it('rejects a sender outside the execution scopes', async () => {
+      const inScopeSender = await AztecAddress.random();
+      const outOfScopeSender = await AztecAddress.random();
+      const recipient = await AztecAddress.random();
+      const oracle = makeOracle({ scopes: [inScopeSender] });
+
+      await expect(oracle.getAppTaggingSecret(outOfScopeSender, recipient)).rejects.toThrow(
+        'is not in the allowed scopes list',
+      );
+    });
+
+    it('returns a secret for a sender inside the execution scopes', async () => {
+      const senderCompleteAddress = await CompleteAddress.random();
+      const sender = senderCompleteAddress.address;
+      const recipient = (await CompleteAddress.random()).address;
+
+      const addressStore = mock<AddressStore>();
+      addressStore.getCompleteAddress.mockResolvedValue(senderCompleteAddress);
+      const keyStore = mock<KeyStore>();
+      keyStore.getMasterIncomingViewingSecretKey.mockResolvedValue(GrumpkinScalar.random());
+
+      const oracle = makeOracle({ scopes: [sender], addressStore, keyStore });
+
+      const result = await oracle.getAppTaggingSecret(sender, recipient);
+      expect(result.isSome()).toBe(true);
+    });
+  });
+
   describe('resolveTaggingStrategy', () => {
     let sender: AztecAddress;
     let recipient: AztecAddress;

yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts

@@ -36,6 +36,7 @@ import {
   type TaggingSecretStrategy,
 } from '../../hooks/resolve_tagging_secret_strategy.js';
 import { NoteService } from '../../notes/note_service.js';
+import { assertAllowedScope } from '../../storage/allowed_scopes.js';
 import type { SenderTaggingStore } from '../../storage/tagging_store/sender_tagging_store.js';
 import { syncSenderTaggingIndexes } from '../../tagging/index.js';
 import type { ExecutionNoteCache } from '../execution_note_cache.js';
@@ -321,7 +322,17 @@ export class PrivateExecutionOracle extends UtilityExecutionOracle implements IP
    * @returns The app tagging secret, or `None` if the recipient is invalid.
    */
   public async getAppTaggingSecret(sender: AztecAddress, recipient: AztecAddress): Promise<Option<Fr>> {
-    const extendedSecret = await this.#calculateAppTaggingSecret(this.contractAddress, sender, recipient);
+    assertAllowedScope(sender, this.scopes);
+
+    const senderCompleteAddress = await this.getCompleteAddressOrFail(sender);
+    const senderIvsk = await this.keyStore.getMasterIncomingViewingSecretKey(sender);
+    const extendedSecret = await AppTaggingSecret.computeViaEcdh(
+      senderCompleteAddress,
+      senderIvsk,
+      recipient,
+      this.contractAddress,
+      recipient,
+    );
 
     if (!extendedSecret) {
       this.logger.warn(`Computing a tagging secret for invalid recipient ${recipient} - returning no secret`, {
@@ -354,12 +365,6 @@ export class PrivateExecutionOracle extends UtilityExecutionOracle implements IP
     return index;
   }
 
-  async #calculateAppTaggingSecret(contractAddress: AztecAddress, sender: AztecAddress, recipient: AztecAddress) {
-    const senderCompleteAddress = await this.getCompleteAddressOrFail(sender);
-    const senderIvsk = await this.keyStore.getMasterIncomingViewingSecretKey(sender);
-    return AppTaggingSecret.computeViaEcdh(senderCompleteAddress, senderIvsk, recipient, contractAddress, recipient);
-  }
-
   async #getIndexToUseForSecret(secret: AppTaggingSecret): Promise<number> {
     // If we have the tagging index in the cache, we use it. If not we obtain it from the execution data provider.
     const lastUsedIndexInTx = this.taggingIndexCache.getLastUsedIndex(secret);

yarn-project/wallet-sdk/src/base-wallet/base_wallet.ts

@@ -125,8 +125,15 @@ export abstract class BaseWallet implements Wallet {
     protected log = createLogger('wallet-sdk:base_wallet'),
   ) {}
 
-  protected scopesFrom(from: AztecAddress | NoFrom, additionalScopes: AztecAddress[] = []): AztecAddress[] {
-    const allScopes = from === NO_FROM ? additionalScopes : [from, ...additionalScopes];
+  protected scopesFrom(
+    from: AztecAddress | NoFrom,
+    additionalScopes: AztecAddress[],
+    sendMessagesAs: AztecAddress | undefined,
+  ): AztecAddress[] {
+    // The sendMessagesAs account must be in scope so that its tagging secrets can be accessed.
+    const tagSenderScopes = sendMessagesAs ? [sendMessagesAs] : [];
+    const baseScopes = from === NO_FROM ? [] : [from];
+    const allScopes = [...baseScopes, ...additionalScopes, ...tagSenderScopes];
     const scopeSet = new Set(allScopes.map(address => address.toString()));
     return [...scopeSet].map(AztecAddress.fromStringUnsafe);
   }
@@ -402,7 +409,7 @@ export abstract class BaseWallet implements Wallet {
       simulatePublic: true,
       skipTxValidation: opts.skipTxValidation,
       skipFeeEnforcement: opts.skipFeeEnforcement,
-      scopes: this.scopesFrom(opts.from, opts.additionalScopes),
+      scopes: this.scopesFrom(opts.from, opts.additionalScopes ?? [], opts.sendMessagesAs),
       senderForTags: this.senderForTagsFrom(opts.from, opts.sendMessagesAs),
       overrides: opts.overrides,
     });
@@ -498,7 +505,7 @@ export abstract class BaseWallet implements Wallet {
     return this.pxe.profileTx(txRequest, {
       profileMode: opts.profileMode,
       skipProofGeneration: opts.skipProofGeneration ?? true,
-      scopes: this.scopesFrom(opts.from, opts.additionalScopes),
+      scopes: this.scopesFrom(opts.from, opts.additionalScopes ?? [], opts.sendMessagesAs),
       senderForTags: this.senderForTagsFrom(opts.from, opts.sendMessagesAs),
     });
   }
@@ -515,7 +522,7 @@ export abstract class BaseWallet implements Wallet {
     });
     const txRequest = await this.createTxExecutionRequestFromPayloadAndFee(executionPayload, opts.from, feeOptions);
     const provenTx = await this.pxe.proveTx(txRequest, {
-      scopes: this.scopesFrom(opts.from, opts.additionalScopes),
+      scopes: this.scopesFrom(opts.from, opts.additionalScopes ?? [], opts.sendMessagesAs),
       senderForTags: this.senderForTagsFrom(opts.from, opts.sendMessagesAs),
     });
     const offchainOutput = extractOffchainOutput(

yarn-project/wallets/src/embedded/embedded_wallet.test.ts

@@ -42,7 +42,7 @@ describe('EmbeddedWallet', () => {
   });
 
   describe('sendTx', () => {
-    it('passes sendMessagesAs as senderForTags to PXE simulation', async () => {
+    it('passes sendMessagesAs as senderForTags and includes it in scopes for PXE simulation', async () => {
       getPredictedMinFees.mockResolvedValue([new GasFees(2, 2)]);
       getNodeInfo.mockResolvedValue({
         l1ChainId: 1,
@@ -69,7 +69,10 @@ describe('EmbeddedWallet', () => {
 
       expect(simulateTx).toHaveBeenCalledWith(
         expect.anything(),
-        expect.objectContaining({ senderForTags: sendMessagesAs }),
+        expect.objectContaining({
+          senderForTags: sendMessagesAs,
+          scopes: expect.arrayContaining([sendMessagesAs]),
+        }),
       );
     });
 

yarn-project/wallets/src/embedded/embedded_wallet.ts

@@ -329,7 +329,7 @@ export class EmbeddedWallet extends BaseWallet {
     opts: SimulateViaEntrypointOptions,
   ): Promise<TxSimulationResultWithAppOffset> {
     const { from, feeOptions, additionalScopes, skipTxValidation, skipFeeEnforcement, sendMessagesAs } = opts;
-    const scopes = this.scopesFrom(from, additionalScopes);
+    const scopes = this.scopesFrom(from, additionalScopes ?? [], sendMessagesAs);
 
     const feeExecutionPayload = await feeOptions.walletFeePaymentMethod?.getExecutionPayload();
     const finalExecutionPayload = feeExecutionPayload