fix: privacy leak in private refund (#7358)

Fixes #7321

noir-projects/noir-contracts/contracts/private_fpc_contract/src/main.nr

@@ -1,7 +1,7 @@
 mod lib;
 
 contract PrivateFPC {
-    use dep::aztec::protocol_types::{abis::log_hash::LogHash, address::AztecAddress};
+    use dep::aztec::protocol_types::{abis::log_hash::LogHash, address::AztecAddress, hash::poseidon2_hash};
     use dep::aztec::state_vars::SharedImmutable;
     use dep::private_token::PrivateToken;
     use crate::lib::emit_randomness_as_unencrypted_log;
@@ -20,19 +20,23 @@ contract PrivateFPC {
     }
 
     #[aztec(private)]
-    fn fund_transaction_privately(amount: Field, asset: AztecAddress, randomness: Field) {
+    fn fund_transaction_privately(amount: Field, asset: AztecAddress, user_randomness: Field) {
         assert(asset == storage.other_asset.read_private());
         // convince the FPC we are not cheating
-        context.push_nullifier(randomness, 0);
+        context.push_nullifier(user_randomness, 0);
 
-        // allow the FPC to reconstruct their fee note
-        emit_randomness_as_unencrypted_log(&mut context, randomness);
+        // We use different randomness for fee payer to prevent a potential privay leak (see impl
+        // of PrivatelyRefundable for TokenNote for details).
+        let fee_payer_randomness = poseidon2_hash([user_randomness]);
+        // We emit fee payer randomness to ensure FPC admin can reconstruct their fee note
+        emit_randomness_as_unencrypted_log(&mut context, fee_payer_randomness);
 
         PrivateToken::at(asset).setup_refund(
             storage.admin_npk_m_hash.read_private(),
             context.msg_sender(),
             amount,
-            randomness
+            user_randomness,
+            fee_payer_randomness
         ).call(&mut context);
         context.set_as_fee_payer();
     }

noir-projects/noir-contracts/contracts/private_token_contract/src/main.nr

@@ -160,7 +160,8 @@ contract PrivateToken {
         fee_payer_npk_m_hash: Field, // NpkMHash of the entity which will receive the fee note.
         user: AztecAddress, // A user for which we are setting up the fee refund.
         funded_amount: Field, // The amount the user funded the fee payer with (represents fee limit).
-        randomness: Field // A randomness to mix in with the generated notes.
+        user_randomness: Field, // A randomness to mix in with the generated refund note for the sponsored user.
+        fee_payer_randomness: Field // A randomness to mix in with the generated fee note for the fee payer.
     ) {
         // 1. This function is called by fee paying contract (fee_payer) when setting up a refund so we need to support
         // the authwit flow here and check that the user really permitted fee_payer to set up a refund on their behalf.
@@ -183,7 +184,8 @@ contract PrivateToken {
             fee_payer_npk_m_hash,
             user_npk_m_hash,
             funded_amount,
-            randomness
+            user_randomness,
+            fee_payer_randomness
         );
 
         // 5. Set the public teardown function to `complete_refund(...)`. Public teardown is the only time when a public

noir-projects/noir-contracts/contracts/private_token_contract/src/test/basic.nr

@@ -40,15 +40,17 @@ unconstrained fn setup_refund_success() {
     let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(true);
 
     let funded_amount = 1_000;
-    let refund_nonce = 42;
+    let user_randomness = 42;
+    let fee_payer_randomness = 123;
     let mut context = env.private();
     let recipient_npk_m_hash = context.get_header().get_npk_m_hash(&mut context, recipient);
 
     let setup_refund_from_call_interface = PrivateToken::at(token_contract_address).setup_refund(
         recipient_npk_m_hash, // fee payer
         owner, // sponsored user
         funded_amount,
-        refund_nonce
+        user_randomness,
+        fee_payer_randomness
     );
 
     authwit_cheatcodes::add_private_authwit_from_call_interface(owner, recipient, setup_refund_from_call_interface);
@@ -70,7 +72,7 @@ unconstrained fn setup_refund_success() {
         &mut TokenNote {
         amount: U128::from_integer(funded_amount - 1),
         npk_m_hash: owner_npk_m_hash,
-        randomness: refund_nonce,
+        randomness: user_randomness,
         header: NoteHeader::empty()
     },
         PrivateToken::storage().balances.slot,
@@ -80,7 +82,7 @@ unconstrained fn setup_refund_success() {
         &mut TokenNote {
         amount: U128::from_integer(1),
         npk_m_hash: recipient_npk_m_hash,
-        randomness: refund_nonce,
+        randomness: fee_payer_randomness,
         header: NoteHeader::empty()
     },
         PrivateToken::storage().balances.slot,

noir-projects/noir-contracts/contracts/private_token_contract/src/types/token_note.nr

@@ -19,7 +19,8 @@ trait PrivatelyRefundable {
         fee_payer_npk_m_hash: Field,
         user_npk_m_hash: Field,
         funded_amount: Field,
-        randomness: Field
+        user_randomness: Field,
+        fee_payer_randomness: Field
     ) -> (EmbeddedCurvePoint, EmbeddedCurvePoint);
 
     fn complete_refund(
@@ -155,8 +156,8 @@ impl OwnedNote for TokenNote {
  * deduce what n is. This is the discrete log problem.
  *
  * However we can still perform addition/subtraction on points! That is why we generate those two points, which are:
- * incomplete_fee_payer_point := (fee_payer_npk + randomness) * G
- * incomplete_user_point := (user_npk + funded_amount + randomness) * G
+ * incomplete_fee_payer_point := (fee_payer_npk + fee_payer_randomness) * G
+ * incomplete_user_point := (user_npk + funded_amount + user_randomness) * G
  *
  * where `funded_amount` is the total amount in tokens that the sponsored user initially supplied, from which the transaction fee will be subtracted.
  *
@@ -167,25 +168,37 @@ impl OwnedNote for TokenNote {
  * Then we arrive at the final points via addition/subtraction of that transaction fee point:
  *
  * fee_payer_point := incomplete_fee_payer_point + fee_point
- *                      = (fee_payer_npk + randomness) * G + transaction_fee * G
- *                      = (fee_payer_npk + randomness + transaction_fee) * G
+ *                      = (fee_payer_npk + fee_payer_randomness) * G + transaction_fee * G
+ *                      = (fee_payer_npk + fee_payer_randomness + transaction_fee) * G
  *
  * user_point := incomplete_user_point - fee_point
- *                       = (user_npk + funded_amount + randomness) * G - transaction_fee * G
- *                       = (user_npk + randomness + (funded_amount - transaction_fee)) * G
+ *                       = (user_npk + funded_amount + user_randomness) * G - transaction_fee * G
+ *                       = (user_npk + user_randomness + (funded_amount - transaction_fee)) * G
  * 
  * When we return the x-coordinate of those points, it identically matches the note_content_hash of (and therefore *is*) notes like:
  * {
  *     amount: (funded_amount - transaction_fee),
  *     npk_m_hash: user_npk,
- *     randomness: randomness
+ *     randomness: user_randomness
  * }
+ *
+ * Why do we need different randomness for the user and the fee payer notes?
+ * --> This is because if the randomness values were the same we could fingerprint the user by doing the following:
+ *      1) randomness_influence = incomplete_fee_payer_point - G * fee_payer_npk =
+ *                              = (fee_payer_npk + randomness) * G - G * fee_payer_npk = randomness * G
+ *      2) user_fingerprint = incomplete_user_point - G * funded_amount - randomness_influence =
+ *                          = (user_npk + funded_amount + randomness) * G - funded_amount * G - randomness * G =
+ *                          = user_npk * G
+ *      3) Then the second time the user would use this fee paying contract we would recover the same fingerprint and
+ *         link that the 2 transactions were made by the same user. Given that it's expected that only a limited set
+ *         of fee paying contracts will be used and they will be known searching for fingerprints by trying different
+ *         fee payer npk values of these known contracts is a feasible attack.
  */
 impl PrivatelyRefundable for TokenNote {
-    fn generate_refund_points(fee_payer_npk_m_hash: Field, user_npk_m_hash: Field, funded_amount: Field, randomness: Field) -> (EmbeddedCurvePoint, EmbeddedCurvePoint) {
+    fn generate_refund_points(fee_payer_npk_m_hash: Field, user_npk_m_hash: Field, funded_amount: Field, user_randomness: Field, fee_payer_randomness: Field) -> (EmbeddedCurvePoint, EmbeddedCurvePoint) {
         // 1. To be able to multiply generators with randomness and npk_m_hash using barretneberg's (BB) blackbox function we
         // first need to convert the fields to high and low limbs.
-        let (randomness_lo, randomness_hi) = decompose(randomness);
+        let (fee_payer_randomness_lo, fee_payer_randomness_hi) = decompose(fee_payer_randomness);
         let (fee_payer_npk_m_hash_lo, fee_payer_npk_m_hash_hi) = decompose(fee_payer_npk_m_hash);
 
         // 2. Now that we have correct representationsn of fee payer and randomness we can compute `G ^ (fee_payer_npk + randomness)`
@@ -196,12 +209,13 @@ impl PrivatelyRefundable for TokenNote {
                 hi: fee_payer_npk_m_hash_hi
             },
             EmbeddedCurveScalar {
-                lo: randomness_lo,
-                hi: randomness_hi
+                lo: fee_payer_randomness_lo,
+                hi: fee_payer_randomness_hi
             }]
         );
 
         // 3. We do the necessary conversion for values relevant for the sponsored user point.
+        let (user_randomness_lo, user_randomness_hi) = decompose(user_randomness);
         // TODO(#7324): representing user with their npk_m_hash here does not work with key rotation
         let (user_lo, user_hi) = decompose(user_npk_m_hash);
         let (funded_amount_lo, funded_amount_hi) = decompose(funded_amount);
@@ -218,8 +232,8 @@ impl PrivatelyRefundable for TokenNote {
                 hi: funded_amount_hi
             },
             EmbeddedCurveScalar {
-                lo: randomness_lo,
-                hi: randomness_hi
+                lo: user_randomness_lo,
+                hi: user_randomness_hi
             }]
         );
 

yarn-project/end-to-end/src/e2e_fees/private_refunds.test.ts

@@ -8,6 +8,7 @@ import {
 } from '@aztec/aztec.js';
 import { Fr, type GasSettings } from '@aztec/circuits.js';
 import { FunctionSelector, FunctionType } from '@aztec/foundation/abi';
+import { poseidon2Hash } from '@aztec/foundation/crypto';
 import { type PrivateFPCContract, PrivateTokenContract } from '@aztec/noir-contracts.js';
 
 import { expectMapping } from '../fixtures/utils.js';
@@ -53,7 +54,8 @@ describe('e2e_fees/private_refunds', () => {
     // TODO(#7324): The values in complete address are currently not updated after the keys are rotated so this does
     // not work with key rotation as the key might be the old one and then we would fetch a new one in the contract.
     const bobNpkMHash = t.bobWallet.getCompleteAddress().publicKeys.masterNullifierPublicKey.hash();
-    const randomness = Fr.random();
+    const aliceRandomness = Fr.random(); // Called user_randomness in contracts
+    const bobRandomness = poseidon2Hash([aliceRandomness]); // Called fee_payer_randomness in contracts
 
     // 2. We call arbitrary `private_get_name(...)` function to check that the fee refund flow works.
     const tx = await privateToken.methods
@@ -65,7 +67,8 @@ describe('e2e_fees/private_refunds', () => {
             privateToken.address,
             privateFPC.address,
             aliceWallet,
-            randomness,
+            aliceRandomness,
+            bobRandomness,
             bobNpkMHash, // We use Bob's npk_m_hash in the notes that contain the transaction fee.
           ),
         },
@@ -81,7 +84,7 @@ describe('e2e_fees/private_refunds', () => {
     // TODO(#7324): The values in complete address are currently not updated after the keys are rotated so this does
     // not work with key rotation as the key might be the old one and then we would fetch a new one in the contract.
     const aliceNpkMHash = t.aliceWallet.getCompleteAddress().publicKeys.masterNullifierPublicKey.hash();
-    const aliceRefundNote = new Note([refundNoteValue, aliceNpkMHash, randomness]);
+    const aliceRefundNote = new Note([refundNoteValue, aliceNpkMHash, aliceRandomness]);
 
     // 4. If the refund flow worked it should have added emitted a note hash of the note we constructed above and we
     // should be able to add the note to our PXE. Just calling `pxe.addNote(...)` is enough of a check that the note
@@ -102,7 +105,7 @@ describe('e2e_fees/private_refunds', () => {
     // npk_m_hash (set in the paymentMethod above) and the randomness.
     // Note that FPC emits randomness as unencrypted log and the tx fee is publicly know so Bob is able to reconstruct
     // his note just from on-chain data.
-    const bobFeeNote = new Note([new Fr(tx.transactionFee!), bobNpkMHash, randomness]);
+    const bobFeeNote = new Note([new Fr(tx.transactionFee!), bobNpkMHash, bobRandomness]);
 
     // 6. Once again we add the note to PXE which computes the note hash and checks that it is in the note hash tree.
     await t.bobWallet.addNote(
@@ -144,10 +147,16 @@ class PrivateRefundPaymentMethod implements FeePaymentMethod {
     private wallet: Wallet,
 
     /**
-     * A randomness to mix in with the generated notes.
+     * A randomness to mix in with the generated refund note for the sponsored user.
      * Use this to reconstruct note preimages for the PXE.
      */
-    private randomness: Fr,
+    private userRandomness: Fr,
+
+    /**
+     * A randomness to mix in with the generated fee note for the fee payer.
+     * Use this to reconstruct note preimages for the PXE.
+     */
+    private feePayerRandomness: Fr,
 
     /**
      * The hash of the master nullifier public key that the FPC sends notes it receives to.
@@ -179,8 +188,14 @@ class PrivateRefundPaymentMethod implements FeePaymentMethod {
       caller: this.paymentContract,
       action: {
         name: 'setup_refund',
-        args: [this.feeRecipientNpkMHash, this.wallet.getCompleteAddress().address, maxFee, this.randomness],
-        selector: FunctionSelector.fromSignature('setup_refund(Field,(Field),Field,Field)'),
+        args: [
+          this.feeRecipientNpkMHash,
+          this.wallet.getCompleteAddress().address,
+          maxFee,
+          this.userRandomness,
+          this.feePayerRandomness,
+        ],
+        selector: FunctionSelector.fromSignature('setup_refund(Field,(Field),Field,Field,Field)'),
         type: FunctionType.PRIVATE,
         isStatic: false,
         to: this.asset,
@@ -195,7 +210,7 @@ class PrivateRefundPaymentMethod implements FeePaymentMethod {
         selector: FunctionSelector.fromSignature('fund_transaction_privately(Field,(Field),Field)'),
         type: FunctionType.PRIVATE,
         isStatic: false,
-        args: [maxFee, this.asset, this.randomness],
+        args: [maxFee, this.asset, this.userRandomness],
         returnTypes: [],
       },
     ];

yarn-project/noir-contracts.js/package.json

@@ -74,4 +74,4 @@
   "engines": {
     "node": ">=18"
   }
-}
+}