vuln fix

docs/docs/migration_notes.md

@@ -39,16 +39,20 @@ Token contract diff:
 -        subtracted = subtracted + note.get_amount();
 -    }
 -}
+-assert(minuend >= subtrahend, "Balance too low");
 +let options = NoteGetterOptions::with_filter(filter_notes_min_sum, target_amount).set_limit(max_notes);
 +let notes = self.map.at(owner).pop_notes(options);
 +let mut subtracted = U128::from_integer(0);
 +for i in 0..options.limit {
 +    if i < notes.len() {
 +        let note = notes.get_unchecked(i);
-+        self.map.at(owner).remove(note);
 +        subtracted = subtracted + note.get_amount();
 +    }
 +}
++assert(minuend >= subtrahend, "Balance too low");
+
+Note that the notes are not ensured to be obtained and removed so you have the place checks on the returned notes (e.g. in the example above by checking a sum of balances or by checking that returned note length assert(notes.len() == expected_num_notes)).
+
 
 
 ## 0.47.0

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

@@ -32,7 +32,7 @@ contract Benchmarking {
         let owner_notes = storage.notes.at(owner);
         let mut getter_options = NoteGetterOptions::new();
         let notes = owner_notes.pop_notes(getter_options.set_limit(1).set_offset(index));
-        let note = notes.get_unchecked(0);
+        let note = notes.get(0);
         increment(owner_notes, note.value, owner, outgoing_viewer);
     }
 

noir-projects/noir-contracts/contracts/card_game_contract/src/cards.nr

@@ -121,6 +121,7 @@ impl Deck<&mut PrivateContext> {
     }
 
     pub fn remove_cards<N>(&mut self, cards: [Card; N]) {
+        // TODO(benesjan): try to simplify this func
         let options = NoteGetterOptions::with_filter(filter_cards, cards);
         let notes = self.set.pop_notes(options);
 

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

@@ -149,7 +149,8 @@ contract InclusionProofs {
         let private_values = storage.private_values.at(owner);
         let mut options = NoteGetterOptions::new();
         options = options.set_limit(1);
-        private_values.pop_notes(options);
+        let notes = private_values.pop_notes(options);
+        assert(notes.len() == 1, "note not popped");
     }
     // docs:end:nullify_note
 

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

@@ -173,7 +173,8 @@ contract TokenBlacklist {
             Option::none()
         ).set_limit(1);
 
-        storage.pending_shields.pop_notes(options);
+        let notes = storage.pending_shields.pop_notes(options);
+        assert(notes.len() == 1, "note not popped");
 
         // Add the token note to user's balances set
         let caller = context.msg_sender();

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

@@ -304,7 +304,8 @@ contract Token {
             Option::none()
         ).set_limit(1);
 
-        storage.pending_shields.pop_notes(options);
+        let notes = storage.pending_shields.pop_notes(options);
+        assert(notes.len() == 1, "note not popped");
 
         // Add the token note to user's balances set
         // Note: Using context.msg_sender() as a sender below makes this incompatible with escrows because we send

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

@@ -291,7 +291,8 @@ contract TokenWithRefunds {
             Option::none()
         ).set_limit(1);
 
-        storage.pending_shields.pop_notes(options);
+        let notes = storage.pending_shields.pop_notes(options);
+        assert(notes.len() == 1, "note not popped");
 
         // Add the token note to user's balances set
         // Note: Using context.msg_sender() as a sender below makes this incompatible with escrows because we send

yarn-project/end-to-end/src/e2e_blacklist_token_contract/minting.test.ts

@@ -111,7 +111,7 @@ describe('e2e_blacklist_token_contract mint', () => {
           'The note has been destroyed.',
         );
         await expect(asset.methods.redeem_shield(wallets[0].getAddress(), amount, secret).prove()).rejects.toThrow(
-          `Assertion failed: Attempted to read past end of BoundedVec`,
+          `Assertion failed: note not popped 'notes.len() == 1'`,
         );
       });
 

yarn-project/end-to-end/src/e2e_token_contract/minting.test.ts

@@ -101,7 +101,7 @@ describe('e2e_token_contract minting', () => {
           'The note has been destroyed.',
         );
         await expect(asset.methods.redeem_shield(accounts[0].address, amount, secret).simulate()).rejects.toThrow(
-          `Assertion failed: Attempted to read past end of BoundedVec`,
+          `Assertion failed: note not popped 'notes.len() == 1'`,
         );
       });