Skip to content

Commit

Permalink
fix: check bytes remaining on monero blocks (tari-project#5610)
Browse files Browse the repository at this point in the history
Description
---
Checks the bytes left after deserializing monero pow_data to ensure all
bytes have been used.

Motivation and Context
---
Because the struct is stored as raw vec<u8> in headers, we need to
ensure that all the bytes are used after deserializing to ensure that
malicious nodes dont attcked bytes to change the hash or enlarge the
headers. See TARI-010

How Has This Been Tested?
---
unit tests

Audit Finding Number
---
TARI-010

---------

Co-authored-by: Stan Bondi <sdbondi@users.noreply.github.com>
  • Loading branch information
SWvheerden and sdbondi committed Aug 9, 2023
1 parent f466840 commit 1087fa9
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
10 changes: 9 additions & 1 deletion base_layer/core/src/proof_of_work/monero_rx/pow_data.rs
Expand Up @@ -94,7 +94,15 @@ impl MoneroPowData {
/// Create a new MoneroPowData struct from the given header
pub fn from_header(tari_header: &BlockHeader) -> Result<MoneroPowData, MergeMineError> {
let mut v = tari_header.pow.pow_data.as_slice();
BorshDeserialize::deserialize(&mut v).map_err(|e| MergeMineError::DeserializeError(format!("{:?}", e)))
let pow_data =
BorshDeserialize::deserialize(&mut v).map_err(|e| MergeMineError::DeserializeError(format!("{:?}", e)))?;
if !v.is_empty() {
return Err(MergeMineError::DeserializeError(format!(
"{} bytes leftover after deserialize",
v.len()
)));
}
Ok(pow_data)
}

/// Returns true if the coinbase merkle proof produces the `merkle_root` hash, otherwise false
Expand Down
20 changes: 20 additions & 0 deletions base_layer/core/tests/tests/block_validation.rs
Expand Up @@ -150,6 +150,20 @@ async fn test_monero_blocks() {
},
};

// lets try add some bad data to the block
let mut extra_bytes_block_3 = block_3.clone();
add_bad_monero_data(&mut extra_bytes_block_3, seed2);
match db.add_block(Arc::new(extra_bytes_block_3)) {
Err(ChainStorageError::ValidationError {
source: ValidationError::CustomError(_),
}) => (),
Err(e) => {
panic!("Failed due to other error:{:?}", e);
},
Ok(res) => {
panic!("Block add unexpectedly succeeded with result: {:?}", res);
},
};
// now lets fix the seed, and try again
add_monero_data(&mut block_3, seed2);
// lets break the nonce count
Expand Down Expand Up @@ -200,6 +214,12 @@ fn add_monero_data(tblock: &mut Block, seed_key: &str) {
tblock.header.pow.pow_data = serialized;
}

fn add_bad_monero_data(tblock: &mut Block, seed_key: &str) {
add_monero_data(tblock, seed_key);
// Add some "garbage" bytes to the end of the pow_data
tblock.header.pow.pow_data.extend([1u8; 100]);
}

#[tokio::test]
async fn inputs_are_not_malleable() {
let _ = env_logger::try_init();
Expand Down

0 comments on commit 1087fa9

Please sign in to comment.