Skip to content

Commit

Permalink
a few more safety warnings fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
benesjan committed Jan 9, 2025
1 parent 6f9c946 commit b7b7b66
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@ impl NullifiableNote for SubscriptionNote {

impl SubscriptionNote {
pub fn new(owner: AztecAddress, expiry_block_number: Field, remaining_txs: Field) -> Self {
// We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, so a
// malicious sender could use non-random values to make the note less private. But they already know the full
// note pre-image anyway, and so the recipient already trusts them to not disclose this information. We can
// therefore assume that the sender will cooperate in the random value generation.
let randomness = unsafe { random() };
let randomness = unsafe {
//@safety We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing,
// so a malicious sender could use non-random values to make the note less private. But they already know
// the full note pre-image anyway, and so the recipient already trusts them to not disclose this
// information. We can therefore assume that the sender will cooperate in the random value generation.
random()
};
Self { owner, expiry_block_number, remaining_txs, randomness, header: NoteHeader::empty() }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ contract SchnorrAccount {
let storage = Storage::init(context);
let public_key = storage.signing_public_key.get_note();
// Load auth witness
let witness: [Field; 64] = unsafe { get_auth_witness(outer_hash) };
let witness: [Field; 64] = unsafe {
//@safety The witness is only used as a "magical value" that makes the signature verification below pass. Hence it's safe.
get_auth_witness(outer_hash)
};
let mut signature: [u8; 64] = [0; 64];
for i in 0..64 {
signature[i] = witness[i] as u8;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ contract SchnorrHardcodedAccount {
#[contract_library_method]
fn is_valid_impl(_context: &mut PrivateContext, outer_hash: Field) -> bool {
// Load auth witness and format as an u8 array
let witness: [Field; 64] = unsafe { get_auth_witness(outer_hash) };
let witness: [Field; 64] = unsafe {
//@safety The witness is only used as a "magical value" that makes the signature verification below pass. Hence it's safe.
get_auth_witness(outer_hash)
};
let mut signature: [u8; 64] = [0; 64];
for i in 0..64 {
signature[i] = witness[i] as u8;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ contract SchnorrSingleKeyAccount {

#[contract_library_method]
fn is_valid_impl(context: &mut PrivateContext, outer_hash: Field) -> bool {
let witness = unsafe { get_auth_witness(outer_hash) };
let witness = unsafe {
//@safety The witness is only used as a "magical value" that makes the signature verification
// in `recover_address` and the address check below pass. Hence it's safe.
get_auth_witness(outer_hash)
};
recover_address(outer_hash, witness).eq(context.this_address())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,13 @@ impl Eq for TokenNote {

impl OwnedNote for TokenNote {
fn new(amount: U128, owner: AztecAddress) -> Self {
// We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, so a
// malicious sender could use non-random values to make the note less private. But they already know the full
// note pre-image anyway, and so the recipient already trusts them to not disclose this information. We can
// therefore assume that the sender will cooperate in the random value generation.
let randomness = unsafe { random() };
let randomness = unsafe {
//@safety We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing,
// so a malicious sender could use non-random values to make the note less private. But they already know
// the full note pre-image anyway, and so the recipient already trusts them to not disclose this
// information. We can therefore assume that the sender will cooperate in the random value generation.
random()
};
Self { amount, owner, randomness, header: NoteHeader::empty() }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,13 @@ impl Eq for TokenNote {

impl OwnedNote for TokenNote {
fn new(amount: U128, owner: AztecAddress) -> Self {
// We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, so a
// malicious sender could use non-random values to make the note less private. But they already know the full
// note pre-image anyway, and so the recipient already trusts them to not disclose this information. We can
// therefore assume that the sender will cooperate in the random value generation.
let randomness = unsafe { random() };
let randomness = unsafe {
//@safety We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing,
// so a malicious sender could use non-random values to make the note less private. But they already know
// the full note pre-image anyway, and so the recipient already trusts them to not disclose this
// information. We can therefore assume that the sender will cooperate in the random value generation.
random()
};
Self { amount, owner, randomness, header: NoteHeader::empty() }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,13 @@ contract Token {
// We create a setup payload with unpopulated/zero `amount` for 'to'
// TODO(#7775): Manually fetching the randomness here is not great. If we decide to include randomness in all
// notes we could just inject it in macros.
let note_randomness = unsafe { random() };
let note_randomness = unsafe {
//@safety We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing,
// so a malicious sender could use non-random values to make the note less private. But they already know
// the full note pre-image anyway, and so the recipient already trusts them to not disclose this
// information. We can therefore assume that the sender will cooperate in the random value generation.
random()
};
let note_setup_payload = UintNote::setup_payload().new(to, note_randomness, to_note_slot);

// We get the keys and encrypt the log of the note
Expand Down

0 comments on commit b7b7b66

Please sign in to comment.