Skip to content

Commit

Permalink
Audit: Static variable size for hash parameters (#56)
Browse files Browse the repository at this point in the history 
* fix: static variables size

* fix: remove size from hash input

* fix: changing hash configuration

* fix: size definition for messageBody

* fix: remove unecessary prints

* fix: padding correction for message body

* test: example test

* fix: validators hashes
  • Loading branch information
@JordyRo1
JordyRo1 authored Jul 18, 2024
1 parent bd47648 commit 4a39e90
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 64 deletions.
10 changes: 2 additions & 8 deletions contracts/src/contracts/isms/multisig/validator_announce.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,8 @@ pub mod validator_announce {
fn domain_hash(self: @ContractState) -> u256 {
let mailbox_address: felt252 = self.mailboxclient.mailbox().try_into().unwrap();
let mut input: Array<ByteData> = array![
ByteData {
value: self.mailboxclient.get_local_domain().into(),
size: u64_word_size(self.mailboxclient.get_local_domain().into()).into()
},
ByteData {
value: mailbox_address.try_into().unwrap(),
size: u256_word_size(mailbox_address.try_into().unwrap()).into()
},
ByteData { value: self.mailboxclient.get_local_domain().into(), size: 4 },
ByteData { value: mailbox_address.try_into().unwrap(), size: 32 },
ByteData { value: HYPERLANE_ANNOUNCEMENT.into(), size: 22 }
];
reverse_endianness(compute_keccak(input.span()))
Expand Down
17 changes: 4 additions & 13 deletions contracts/src/contracts/libs/checkpoint_lib.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,8 @@ pub mod checkpoint_lib {
let domain_hash = CheckpointLib::domain_hash(_origin, _origin_merkle_tree_hook);
let mut input: Array<ByteData> = array![
ByteData { value: domain_hash.into(), size: HASH_SIZE },
ByteData {
value: _checkpoint_root.into(),
size: u256_word_size(_checkpoint_root.into()).into()
},
ByteData {
value: _checkpoint_index.into(),
size: u64_word_size(_checkpoint_index.into()).into()
},
ByteData { value: _checkpoint_root.into(), size: 32 },
ByteData { value: _checkpoint_index.into(), size: 4 },
ByteData { value: _message_id.into(), size: HASH_SIZE },
];
to_eth_signature(reverse_endianness(compute_keccak(input.span())))
Expand All @@ -69,11 +63,8 @@ pub mod checkpoint_lib {
/// u256 - The domain hash.
fn domain_hash(_origin: u32, _origin_merkle_tree_hook: u256) -> u256 {
let mut input: Array<ByteData> = array![
ByteData { value: _origin.into(), size: u64_word_size(_origin.into()).into() },
ByteData {
value: _origin_merkle_tree_hook.into(),
size: u256_word_size(_origin_merkle_tree_hook).into()
},
ByteData { value: _origin.into(), size: 4 },
ByteData { value: _origin_merkle_tree_hook.into(), size: 32 },
ByteData { value: HYPERLANE.into(), size: 9 }
];
reverse_endianness(compute_keccak(input.span()))
Expand Down
145 changes: 116 additions & 29 deletions contracts/src/contracts/libs/message.cairo
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use alexandria_bytes::{Bytes, BytesTrait, BytesStore};
use alexandria_math::BitShift;
use hyperlane_starknet::utils::keccak256::{
reverse_endianness, compute_keccak, ByteData, u256_word_size, u64_word_size, ADDRESS_SIZE
reverse_endianness, compute_keccak, ByteData, u256_word_size, u64_word_size, ADDRESS_SIZE,
u128_mask,
};
use starknet::{ContractAddress, contract_address_const};

Expand Down Expand Up @@ -52,41 +54,126 @@ pub impl MessageImpl of MessageTrait {
let recipient: felt252 = _message.recipient.into();

let mut input: Array<ByteData> = array![
ByteData {
value: _message.version.into(), size: u64_word_size(_message.version.into()).into()
},
ByteData {
value: _message.nonce.into(), size: u64_word_size(_message.nonce.into()).into()
},
ByteData {
value: _message.origin.into(), size: u64_word_size(_message.origin.into()).into()
},
ByteData { value: sender.into(), size: ADDRESS_SIZE },
ByteData {
value: _message.destination.into(),
size: u64_word_size(_message.destination.into()).into()
},
ByteData { value: recipient.into(), size: ADDRESS_SIZE },
ByteData {
value: _message.body.size().into(),
size: u64_word_size(_message.body.size().into()).into()
},
ByteData { value: _message.version.into(), size: 1 },
ByteData { value: _message.nonce.into(), size: 4 },
ByteData { value: _message.origin.into(), size: 4 },
ByteData { value: sender.into(), size: 32 },
ByteData { value: _message.destination.into(), size: 4 },
ByteData { value: recipient.into(), size: 32 },
];
let message_data = _message.clone().body.data();
let finalized_input = MessageImpl::append_span_u128_to_byte_data(
input, message_data.span(), _message.clone().body.size()
);
(reverse_endianness(compute_keccak(finalized_input)), _message)
}

let mut message_data = _message.clone().body.data();
fn append_span_u128_to_byte_data(
mut _input: Array<ByteData>, _to_append: Span<u128>, size: u32
) -> Span<ByteData> {
let mut cur_idx = 0;
let range = size / 16;
loop {
match message_data.pop_front() {
Option::Some(data) => {
input
if (cur_idx == range) {
if (size % 16 == 0) {
break;
} else {
let remaining_size = size - cur_idx * 16;
let mask = u128_mask(remaining_size.try_into().unwrap());
_input
.append(
ByteData {
value: data.into(), size: u256_word_size(data.into()).into()
value: (BitShift::shr(
*_to_append.at(cur_idx), ((16 - remaining_size) * 8).into()
)
& mask)
.into(),
size: remaining_size
}
);
},
Option::None(_) => { break (); }
};
break;
}
}
_input.append(ByteData { value: (*_to_append.at(cur_idx)).into(), size: 16 });
cur_idx += 1;
};
(reverse_endianness(compute_keccak(input.span())), _message)
_input.span()
}
}


#[cfg(test)]
mod tests {
use super::{MessageImpl, ByteData};

#[test]
fn test_append_u128_to_byte_array() {
let input: Array<u128> = array![
0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb,
0xcc000000000000000000000000000000
];
let output_array = MessageImpl::append_span_u128_to_byte_data(array![], input.span(), 33);
assert_eq!(
output_array,
array![
ByteData { value: 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, size: 16 },
ByteData { value: 0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, size: 16 },
ByteData { value: 0xcc, size: 1 }
]
.span()
);

let input: Array<u128> = array![
0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb,
0xcccccccccccccccccccccccccccccccc
];
let output_array = MessageImpl::append_span_u128_to_byte_data(array![], input.span(), 48);
assert_eq!(
output_array,
array![
ByteData { value: 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, size: 16 },
ByteData { value: 0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, size: 16 },
ByteData { value: 0xcccccccccccccccccccccccccccccccc, size: 16 }
]
.span()
);

let input: Array<u128> = array![
0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb,
0xcccccccccccccccccccccccccccccc00
];
let output_array = MessageImpl::append_span_u128_to_byte_data(array![], input.span(), 47);
assert_eq!(
output_array,
array![
ByteData { value: 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, size: 16 },
ByteData { value: 0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, size: 16 },
ByteData { value: 0xcccccccccccccccccccccccccccccc, size: 15 }
]
.span()
);

let input: Array<u128> = array![
0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb,
0xcccccccccccccccccccccccccc000000
];
let output_array = MessageImpl::append_span_u128_to_byte_data(array![], input.span(), 45);
assert_eq!(
output_array,
array![
ByteData { value: 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, size: 16 },
ByteData { value: 0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, size: 16 },
ByteData { value: 0xcccccccccccccccccccccccccc, size: 13 }
]
.span()
);

let input: Array<u128> = array![0x12345678000000000000000000000000];
let output_array = MessageImpl::append_span_u128_to_byte_data(array![], input.span(), 4);
assert_eq!(output_array, array![ByteData { value: 0x12345678, size: 4 }].span());
}
}
24 changes: 12 additions & 12 deletions contracts/src/tests/setup.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,13 +406,13 @@ pub fn build_fake_messageid_metadata(

// Configuration from the main cairo repo: https://github.com/starkware-libs/cairo/blob/main/corelib/src/test/secp256k1_test.cairo
pub fn get_message_and_signature() -> (u256, Array<felt252>, Array<EthSignature>) {
let msg_hash = 0x91745536C898FFA3831357EDCE68E1584287129C8E3AE9A065AC5B31AA355E8C;
let msg_hash = 0xD2B30308834E6E76F50891F0B45E742BE4F4A163BAF697B465DD14C968777AD0;
let validators_array: Array<felt252> = array![
0xcc802e4ba45ee188d1955bba8d3ac4d64efe561e.try_into().unwrap(),
0x3f20deb9b7de97fe4d1c07cdd189eafece816489.try_into().unwrap(),
0x58adaf0255f4626f75cdeb5f75f0c5a309847132.try_into().unwrap(),
0x4e30030da04bf8f52a4ff5cf0fc2ef7bd7d4a840.try_into().unwrap(),
0x3c521ab0c2ade5410483d95e40a7a1f956b87fa9.try_into().unwrap(),
0xfa3ed5df8369fb40e75978937607b6f0c0e04fb8.try_into().unwrap(),
0x2f7008b31a614685a1d24a684827cf05f15dc17a.try_into().unwrap(),
0x108e9c0c2a24c02d70b224ee9dd97136cdc1e072.try_into().unwrap(),
0x7851fee3ec28606dfb18041396876524e8fa6256.try_into().unwrap(),
0xaef2f8fa001982939416fd49e6e533e9fba65a1b.try_into().unwrap(),
];
let signatures = array![
EthSignature {
Expand Down Expand Up @@ -509,13 +509,13 @@ pub fn build_fake_merkle_metadata(

// Configuration from the main cairo repo: https://github.com/starkware-libs/cairo/blob/main/corelib/src/test/secp256k1_test.cairo
pub fn get_merkle_message_and_signature() -> (u256, Array<felt252>, Array<EthSignature>) {
let msg_hash = 0x59771D2D2620ABA94DF0A5E7EE7B1FE0D437D7710D43847097A471A4101AE2D4;
let msg_hash = 0xC9E505E3C9DDD36638A10238CAD43278311CDF09894EF7EFBE6AFBF510507907;
let validators_array: Array<felt252> = array![
0xf4a1ea6c9380f182730df38c8f2347ac95a15839.try_into().unwrap(),
0xe8d8f27c89192757cd4be34d5cf5eda849b85929.try_into().unwrap(),
0x18836f7e081a0a971178413c428230dc3b5d62c5.try_into().unwrap(),
0x77a0172ee7de58a66da42fee91c0cb92775cf4e1.try_into().unwrap(),
0x7058f79d7b18f6ee3808139a0b9ccf9fd8883574.try_into().unwrap(),
0x2614ea33eaf750585f8ae3b59a45b2c800b952ed.try_into().unwrap(),
0xc69bb3f1bedb35543f1c28c0ff4f6428622068d3.try_into().unwrap(),
0x7f2c0c840dd8855e0625f1e1ede157d4e09bc9c4.try_into().unwrap(),
0x6d937801434bbf68eb109d3ab2b103a64afcbac2.try_into().unwrap(),
0xfe5fb670978ee5c686db8a7180509b5682f9797d.try_into().unwrap(),
];
let signatures = array![
EthSignature {
Expand Down
4 changes: 2 additions & 2 deletions contracts/src/tests/test_validator_announce.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub const TEST_STARKNET_DOMAIN: u32 = 23448593;
#[test]
fn test_announce() {
let (validator_announce, mut spy) = setup_validator_announce();
let validator_address: EthAddress = 0xe6076407ca06f2b0a0ec716db2b5361beccdcfa8
let validator_address: EthAddress = 0xf85362bdff5a3561481819b8c9010770384aaecf
.try_into()
.unwrap();
let mut _storage_location: Array<felt252> = array![
Expand Down Expand Up @@ -98,7 +98,7 @@ fn test_announce_fails_if_wrong_signer() {
#[should_panic(expected: ('Announce already occured',))]
fn test_announce_fails_if_replay() {
let (validator_announce, _) = setup_validator_announce();
let validator_address: EthAddress = 0xe6076407ca06f2b0a0ec716db2b5361beccdcfa8
let validator_address: EthAddress = 0xf85362bdff5a3561481819b8c9010770384aaecf
.try_into()
.unwrap();
let mut storage_location: Array<felt252> = array![
Expand Down
32 changes: 32 additions & 0 deletions contracts/src/utils/keccak256.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,38 @@ pub fn one_shift_left_bytes_u256(n_bytes: u8) -> u256 {
}
}

/// Shifts equivalent u128 mask for a given number of bytes
/// dev : panics if u128 overflow
///
/// # Arguments
///
/// * `n_bytes` - The number of bytes shift
///
/// # Returns
///
/// u256 representing the associated mask
pub fn u128_mask(n_bytes: u8) -> u128 {
match n_bytes {
0 => 0,
1 => 0xFF,
2 => 0xFFFF,
3 => 0xFFFFFF,
4 => 0xFFFFFFFF,
5 => 0xFFFFFFFFFF,
6 => 0xFFFFFFFFFFFF,
7 => 0xFFFFFFFFFFFFFF,
8 => 0xFFFFFFFFFFFFFFFF,
9 => 0xFFFFFFFFFFFFFFFFFF,
10 => 0xFFFFFFFFFFFFFFFFFFFF,
11 => 0xFFFFFFFFFFFFFFFFFFFFFF,
12 => 0xFFFFFFFFFFFFFFFFFFFFFFFF,
13 => 0xFFFFFFFFFFFFFFFFFFFFFFFFFF,
14 => 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFF,
15 => 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF,
16 => 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF,
_ => core::panic_with_felt252('n_bytes too big'),
}
}

/// Givens a span of ByteData, returns a concatenated string (ByteArray) of the input
///
Expand Down

0 comments on commit 4a39e90

Please sign in to comment.