From cf5dcd9e242edbacf65e35a06e111ebfa415581a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Mon, 30 Mar 2026 19:38:50 +0000 Subject: [PATCH 1/5] fix: prevent oracle failure on tag computation for invalid recipient --- .../aztec-nr/aztec/src/messages/logs/utils.nr | 19 +++++++++++++ .../foundation/src/curves/grumpkin/point.ts | 2 +- .../oracle/private_execution_oracle.ts | 17 +++++++++++- yarn-project/pxe/src/logs/log_service.ts | 14 ++++++++-- ...extended_directional_app_tagging_secret.ts | 27 +++++++++++++------ 5 files changed, 67 insertions(+), 12 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/messages/logs/utils.nr b/noir-projects/aztec-nr/aztec/src/messages/logs/utils.nr index 526acc55b4d3..1dcbf928ea16 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/logs/utils.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/logs/utils.nr @@ -21,7 +21,9 @@ pub(crate) fn compute_discovery_tag(recipient: AztecAddress) -> Field { } mod test { + use crate::oracle::notes::set_sender_for_tags; use crate::protocol::{address::AztecAddress, traits::FromField}; + use crate::test::helpers::test_environment::TestEnvironment; use super::compute_discovery_tag; use std::test::OracleMock; @@ -42,4 +44,21 @@ mod test { let _ = OracleMock::mock("aztec_prv_getNextAppTagAsSender").returns(expected_tag); assert_eq(compute_discovery_tag(recipient), expected_tag); } + + #[test] + unconstrained fn does_not_fail_on_an_invalid_recipient() { + let mut env = TestEnvironment::new(); + + let sender = env.create_light_account(); + + env.private_context(|_context| { + set_sender_for_tags(sender); + + // The recipient is an invalid address + let recipient = AztecAddress { inner: 3 }; + assert(!recipient.is_valid()); + + let _ = compute_discovery_tag(recipient); + }); + } } diff --git a/yarn-project/foundation/src/curves/grumpkin/point.ts b/yarn-project/foundation/src/curves/grumpkin/point.ts index cb486d898188..859aa8492937 100644 --- a/yarn-project/foundation/src/curves/grumpkin/point.ts +++ b/yarn-project/foundation/src/curves/grumpkin/point.ts @@ -65,7 +65,7 @@ export class Point { } /** - * Generate a random Point instance. + * Generate a random Point instance that is in the curve. * * @returns A randomly generated Point instance. */ diff --git a/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts b/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts index beb60ab64971..da59ece16210 100644 --- a/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts +++ b/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts @@ -203,12 +203,27 @@ export class PrivateExecutionOracle extends UtilityExecutionOracle implements IP * @returns An app tag to be used in a log. */ public async getNextAppTagAsSender(sender: AztecAddress, recipient: AztecAddress): Promise { - const extendedSecret = await this.#calculateExtendedDirectionalAppTaggingSecret( + let extendedSecret = await this.#calculateExtendedDirectionalAppTaggingSecret( this.contractAddress, sender, recipient, ); + if (!extendedSecret) { + // We'd only fail to compute an extended secret if the recipient is an invalid address. To prevent + // king-of-the-hill attacks, instead of failing we use a random tag. By including a correct-looking tag in the + // log, the transaction shape is preserved and no privacy is leaked, even if the tag is bogus. + this.logger.warn( + `Computing a tag for invalid recipient ${recipient} - using a random shared secret with index 0 instead`, + { contractAddress: this.contractAddress }, + ); + extendedSecret = new ExtendedDirectionalAppTaggingSecret(Fr.random(), this.contractAddress); + + // We don't bother syncing indices or keeping track of this random shared secret - that'd just create unnecessary + // extra work for us. + return Tag.compute({ extendedSecret, index: 0 }); + } + const index = await this.#getIndexToUseForSecret(extendedSecret); this.logger.debug( `Incrementing tagging index for sender: ${sender}, recipient: ${recipient}, contract: ${this.contractAddress} to ${index}`, diff --git a/yarn-project/pxe/src/logs/log_service.ts b/yarn-project/pxe/src/logs/log_service.ts index 0f830e018b3e..6df008e1b31c 100644 --- a/yarn-project/pxe/src/logs/log_service.ts +++ b/yarn-project/pxe/src/logs/log_service.ts @@ -176,14 +176,24 @@ export class LogService { ); return Promise.all( - deduplicatedSenders.map(sender => { - return ExtendedDirectionalAppTaggingSecret.compute( + deduplicatedSenders.map(async sender => { + const secret = await ExtendedDirectionalAppTaggingSecret.compute( recipientCompleteAddress, recipientIvsk, sender, contractAddress, recipient, ); + + if (!secret) { + // Note that all senders originate from either the SenderAddressBookStore or the KeyStore. + // TODO(F-512): make sure we actually prevent registering invalid senders. + throw new Error( + `Failed to compute a tagging secret for sender ${sender} - this implies this is an invalid address, which should not happen as they have been previously registered in PXE.`, + ); + } + + return secret; }), ); } diff --git a/yarn-project/stdlib/src/logs/extended_directional_app_tagging_secret.ts b/yarn-project/stdlib/src/logs/extended_directional_app_tagging_secret.ts index 0a083ed053bc..b8c4ae05a96d 100644 --- a/yarn-project/stdlib/src/logs/extended_directional_app_tagging_secret.ts +++ b/yarn-project/stdlib/src/logs/extended_directional_app_tagging_secret.ts @@ -23,14 +23,14 @@ import { computeAddressSecret, computePreaddress } from '../keys/derivation.js'; * doesn't seem to be a good way around this. */ export class ExtendedDirectionalAppTaggingSecret { - private constructor( + constructor( public readonly secret: Fr, public readonly app: AztecAddress, ) {} /** * Derives shared tagging secret and from that, the app address and recipient derives the directional app tagging - * secret. + * secret. Returns undefined if `externalAddress` is an invalid address. * * @param localAddress - The complete address of entity A in the shared tagging secret derivation scheme * @param localIvsk - The incoming viewing secret key of entity A @@ -45,8 +45,12 @@ export class ExtendedDirectionalAppTaggingSecret { externalAddress: AztecAddress, app: AztecAddress, recipient: AztecAddress, - ): Promise { + ): Promise { const taggingSecretPoint = await computeSharedTaggingSecret(localAddress, localIvsk, externalAddress); + if (!taggingSecretPoint) { + return undefined; + } + const appTaggingSecret = await poseidon2Hash([taggingSecretPoint.x, taggingSecretPoint.y, app]); const directionalAppTaggingSecret = await poseidon2Hash([appTaggingSecret, recipient]); @@ -63,18 +67,25 @@ export class ExtendedDirectionalAppTaggingSecret { } } -// Returns shared tagging secret computed with Diffie-Hellman key exchange. +// Returns shared tagging secret computed with Diffie-Hellman key exchange, or undefined if `externalAddress` is an +// invalid address. async function computeSharedTaggingSecret( localAddress: CompleteAddress, localIvsk: Fq, externalAddress: AztecAddress, -): Promise { - const knownPreaddress = await computePreaddress(await localAddress.publicKeys.hash(), localAddress.partialAddress); - // TODO: #8970 - Computation of address point from x coordinate might fail - const externalAddressPoint = await externalAddress.toAddressPoint(); +): Promise { // Given A (local complete address) -> B (external address) and h == preaddress // Compute shared secret as S = (h_A + local_ivsk_A) * Addr_Point_B + const knownPreaddress = await computePreaddress(await localAddress.publicKeys.hash(), localAddress.partialAddress); + + // An invalid address has no corresponding address point + if (!(await externalAddress.isValid())) { + return undefined; + } + + const externalAddressPoint = await externalAddress.toAddressPoint(); + // Beware! h_a + local_ivsk_a (also known as the address secret) can lead to an address point with a negative // y-coordinate, since there's two possible candidates computeAddressSecret takes care of selecting the one that // leads to a positive y-coordinate, which is the only valid address point From 84bf132aeb982cfc0582a055e2fc69e56aef09b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Tue, 31 Mar 2026 16:40:57 -0300 Subject: [PATCH 2/5] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jan Beneš --- yarn-project/foundation/src/curves/grumpkin/point.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/foundation/src/curves/grumpkin/point.ts b/yarn-project/foundation/src/curves/grumpkin/point.ts index 859aa8492937..4254c0012407 100644 --- a/yarn-project/foundation/src/curves/grumpkin/point.ts +++ b/yarn-project/foundation/src/curves/grumpkin/point.ts @@ -65,7 +65,7 @@ export class Point { } /** - * Generate a random Point instance that is in the curve. + * Generate a random Point instance that is on the curve. * * @returns A randomly generated Point instance. */ From a1375f19dc5f7e5c91e7a09abc0bd67efcb561f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Wed, 1 Apr 2026 03:26:44 +0200 Subject: [PATCH 3/5] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jan Beneš --- .../oracle/private_execution_oracle.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts b/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts index da59ece16210..2b14d15d04b9 100644 --- a/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts +++ b/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts @@ -203,7 +203,7 @@ export class PrivateExecutionOracle extends UtilityExecutionOracle implements IP * @returns An app tag to be used in a log. */ public async getNextAppTagAsSender(sender: AztecAddress, recipient: AztecAddress): Promise { - let extendedSecret = await this.#calculateExtendedDirectionalAppTaggingSecret( + const extendedSecret = await this.#calculateExtendedDirectionalAppTaggingSecret( this.contractAddress, sender, recipient, @@ -217,11 +217,7 @@ export class PrivateExecutionOracle extends UtilityExecutionOracle implements IP `Computing a tag for invalid recipient ${recipient} - using a random shared secret with index 0 instead`, { contractAddress: this.contractAddress }, ); - extendedSecret = new ExtendedDirectionalAppTaggingSecret(Fr.random(), this.contractAddress); - - // We don't bother syncing indices or keeping track of this random shared secret - that'd just create unnecessary - // extra work for us. - return Tag.compute({ extendedSecret, index: 0 }); + return new Tag(Fr.random()); } const index = await this.#getIndexToUseForSecret(extendedSecret); From 3f5a314322363e4308c80101ae38ad36405ac3ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Wed, 1 Apr 2026 03:28:00 +0200 Subject: [PATCH 4/5] Update yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts --- .../oracle/private_execution_oracle.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts b/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts index 2b14d15d04b9..0d0cea903707 100644 --- a/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts +++ b/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts @@ -214,7 +214,7 @@ export class PrivateExecutionOracle extends UtilityExecutionOracle implements IP // king-of-the-hill attacks, instead of failing we use a random tag. By including a correct-looking tag in the // log, the transaction shape is preserved and no privacy is leaked, even if the tag is bogus. this.logger.warn( - `Computing a tag for invalid recipient ${recipient} - using a random shared secret with index 0 instead`, + `Computing a tag for invalid recipient ${recipient} - returning a random tag instead`, { contractAddress: this.contractAddress }, ); return new Tag(Fr.random()); From e85831be08951f4439229ec4458c944dbdddfffa Mon Sep 17 00:00:00 2001 From: benesjan Date: Wed, 1 Apr 2026 01:53:57 +0000 Subject: [PATCH 5/5] fmt --- .../oracle/private_execution_oracle.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts b/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts index 0d0cea903707..ebeb69b51e36 100644 --- a/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts +++ b/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution_oracle.ts @@ -213,10 +213,9 @@ export class PrivateExecutionOracle extends UtilityExecutionOracle implements IP // We'd only fail to compute an extended secret if the recipient is an invalid address. To prevent // king-of-the-hill attacks, instead of failing we use a random tag. By including a correct-looking tag in the // log, the transaction shape is preserved and no privacy is leaked, even if the tag is bogus. - this.logger.warn( - `Computing a tag for invalid recipient ${recipient} - returning a random tag instead`, - { contractAddress: this.contractAddress }, - ); + this.logger.warn(`Computing a tag for invalid recipient ${recipient} - returning a random tag instead`, { + contractAddress: this.contractAddress, + }); return new Tag(Fr.random()); }