Skip to content

Commit

Permalink
fix: ignore compression of blocks after msg.len in sha256_var (#6206)
Browse files Browse the repository at this point in the history
# Description

Fix an issue where `sha256_var` produces wrong results for messages with
larger paddings

## Problem\*

Resolves #6163 

## Summary\*

`h = sha256_compression(msg_u8_to_u32(msg_block), h)` was run for bytes
after length as well. This block is moved to `if msg_start <
message_size` block


## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
saleel authored Oct 2, 2024
1 parent dfeb1c5 commit 76eec71
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 1 deletion.
36 changes: 35 additions & 1 deletion noir_stdlib/src/hash/sha256.nr
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ pub fn sha256_var<let N: u32>(msg: [u8; N], message_size: u64) -> [u8; 32] {

// If the block is filled, compress it.
// An un-filled block is handled after this loop.
if msg_byte_ptr == BLOCK_SIZE {
if (msg_start < message_size) & (msg_byte_ptr == BLOCK_SIZE) {
h = sha256_compression(msg_u8_to_u32(msg_block), h);
}
}
Expand Down Expand Up @@ -335,4 +335,38 @@ mod tests {
];
assert_eq(sha256_var(input, input.len() as u64), result);
}

#[test]
fn same_msg_len_variable_padding() {
let input = [
29, 81, 165, 84, 243, 114, 101, 37, 242, 146, 127, 99, 69, 145, 39, 72, 213, 39, 253, 179, 218, 37, 217, 201, 172, 93, 198, 50, 249, 70, 15, 30, 162, 112, 187, 40, 140, 9, 236, 53, 32, 44, 38, 163, 113, 254, 192, 197, 44, 89, 71, 130, 169, 242, 17, 211, 214, 72, 19, 178, 186, 168, 147, 127, 99, 101, 252, 227, 8, 147, 150, 85, 97, 158, 17, 107, 218, 244, 82, 113, 247, 91, 208, 214, 60, 244, 87, 137, 173, 201, 130, 18, 66, 56, 198, 149, 207, 189, 175, 120, 123, 224, 177, 167, 251, 159, 143, 110, 68, 183, 189, 70, 126, 32, 35, 164, 44, 30, 44, 12, 65, 18, 62, 239, 242, 2, 248, 104, 2, 178, 64, 28, 126, 36, 137, 24, 14, 116, 91, 98, 90, 159, 218, 102, 45, 11, 110, 223, 245, 184, 52, 99, 59, 245, 136, 175, 3, 72, 164, 146, 145, 116, 22, 66, 24, 49, 193, 121, 3, 60, 37, 41, 97, 3, 190, 66, 195, 225, 63, 46, 3, 118, 4, 208, 15, 1, 40, 254, 235, 151, 123, 70, 180, 170, 44, 172, 90, 4, 254, 53, 239, 116, 246, 67, 56, 129, 61, 22, 169, 213, 65, 27, 216, 116, 162, 239, 214, 207, 126, 177, 20, 100, 25, 48, 143, 84, 215, 70, 197, 53, 65, 70, 86, 172, 61, 62, 9, 212, 167, 169, 133, 41, 126, 213, 196, 33, 192, 238, 0, 63, 246, 215, 58, 128, 110, 101, 92, 3, 170, 214, 130, 149, 52, 81, 125, 118, 233, 3, 118, 193, 104, 207, 120, 115, 77, 253, 191, 122, 0, 107, 164, 207, 113, 81, 169, 36, 201, 228, 74, 134, 131, 218, 178, 35, 30, 216, 101, 2, 103, 174, 87, 95, 50, 50, 215, 157, 5, 210, 188, 54, 211, 78, 45, 199, 96, 121, 241, 241, 176, 226, 194, 134, 130, 89, 217, 210, 186, 32, 140, 39, 91, 103, 212, 26, 87, 32, 72, 144, 228, 230, 117, 99, 188, 50, 15, 69, 79, 179, 50, 12, 106, 86, 218, 101, 73, 142, 243, 29, 250, 122, 228, 233, 29, 255, 22, 121, 114, 125, 103, 41, 250, 241, 179, 126, 158, 198, 116, 209, 65, 94, 98, 228, 175, 169, 96, 3, 9, 233, 133, 214, 55, 161, 164, 103, 80, 85, 24, 186, 64, 167, 92, 131, 53, 101, 202, 47, 25, 104, 118, 155, 14, 12, 12, 25, 116, 45, 221, 249, 28, 246, 212, 200, 157, 167, 169, 56, 197, 181, 4, 245, 146, 1, 140, 234, 191, 212, 228, 125, 87, 81, 86, 119, 30, 63, 129, 143, 32, 96
];

// Prepare inputs of different lengths
let mut input_511 = [0; 511];
let mut input_512 = [0; 512]; // Next block
let mut input_575 = [0; 575];
let mut input_576 = [0; 576]; // Next block
for i in 0..input.len() {
input_511[i] = input[i];
input_512[i] = input[i];
input_575[i] = input[i];
input_576[i] = input[i];
}

// Compute hashes of all inputs (with same message length)
let fixed_length_hash = super::sha256(input);
let var_full_length_hash = sha256_var(input, input.len() as u64);
let var_length_hash_511 = sha256_var(input_511, input.len() as u64);
let var_length_hash_512 = sha256_var(input_512, input.len() as u64);
let var_length_hash_575 = sha256_var(input_575, input.len() as u64);
let var_length_hash_576 = sha256_var(input_576, input.len() as u64);

// All of the above should have produced the same hash
assert_eq(var_full_length_hash, fixed_length_hash);
assert_eq(var_length_hash_511, fixed_length_hash);
assert_eq(var_length_hash_512, fixed_length_hash);
assert_eq(var_length_hash_575, fixed_length_hash);
assert_eq(var_length_hash_576, fixed_length_hash);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "sha256_var_padding_regression"
type = "bin"
authors = [""]
compiler_version = ">=0.34.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
preimage = [29, 81, 165, 84, 243, 114, 101, 37, 242, 146, 127, 99, 69, 145, 39, 72, 213, 39, 253, 179, 218, 37, 217, 201, 172, 93, 198, 50, 249, 70, 15, 30, 162, 112, 187, 40, 140, 9, 236, 53, 32, 44, 38, 163, 113, 254, 192, 197, 44, 89, 71, 130, 169, 242, 17, 211, 214, 72, 19, 178, 186, 168, 147, 127, 99, 101, 252, 227, 8, 147, 150, 85, 97, 158, 17, 107, 218, 244, 82, 113, 247, 91, 208, 214, 60, 244, 87, 137, 173, 201, 130, 18, 66, 56, 198, 149, 207, 189, 175, 120, 123, 224, 177, 167, 251, 159, 143, 110, 68, 183, 189, 70, 126, 32, 35, 164, 44, 30, 44, 12, 65, 18, 62, 239, 242, 2, 248, 104, 2, 178, 64, 28, 126, 36, 137, 24, 14, 116, 91, 98, 90, 159, 218, 102, 45, 11, 110, 223, 245, 184, 52, 99, 59, 245, 136, 175, 3, 72, 164, 146, 145, 116, 22, 66, 24, 49, 193, 121, 3, 60, 37, 41, 97, 3, 190, 66, 195, 225, 63, 46, 3, 118, 4, 208, 15, 1, 40, 254, 235, 151, 123, 70, 180, 170, 44, 172, 90, 4, 254, 53, 239, 116, 246, 67, 56, 129, 61, 22, 169, 213, 65, 27, 216, 116, 162, 239, 214, 207, 126, 177, 20, 100, 25, 48, 143, 84, 215, 70, 197, 53, 65, 70, 86, 172, 61, 62, 9, 212, 167, 169, 133, 41, 126, 213, 196, 33, 192, 238, 0, 63, 246, 215, 58, 128, 110, 101, 92, 3, 170, 214, 130, 149, 52, 81, 125, 118, 233, 3, 118, 193, 104, 207, 120, 115, 77, 253, 191, 122, 0, 107, 164, 207, 113, 81, 169, 36, 201, 228, 74, 134, 131, 218, 178, 35, 30, 216, 101, 2, 103, 174, 87, 95, 50, 50, 215, 157, 5, 210, 188, 54, 211, 78, 45, 199, 96, 121, 241, 241, 176, 226, 194, 134, 130, 89, 217, 210, 186, 32, 140, 39, 91, 103, 212, 26, 87, 32, 72, 144, 228, 230, 117, 99, 188, 50, 15, 69, 79, 179, 50, 12, 106, 86, 218, 101, 73, 142, 243, 29, 250, 122, 228, 233, 29, 255, 22, 121, 114, 125, 103, 41, 250, 241, 179, 126, 158, 198, 116, 209, 65, 94, 98, 228, 175, 169, 96, 3, 9, 233, 133, 214, 55, 161, 164, 103, 80, 85, 24, 186, 64, 167, 92, 131, 53, 101, 202, 47, 25, 104, 118, 155, 14, 12, 12, 25, 116, 45, 221, 249, 28, 246, 212, 200, 157, 167, 169, 56, 197, 181, 4, 245, 146, 1, 140, 234, 191, 212, 228, 125, 87, 81, 86, 119, 30, 63, 129, 143, 32, 96]
result = [205, 74, 73, 134, 202, 93, 199, 152, 171, 244, 133, 193, 132, 40, 42, 9, 248, 11, 99, 200, 135, 58, 220, 227, 45, 253, 183, 241, 69, 69, 80, 219]
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Test to check sha256_var produces same results irrespective of number of padding bytes after message.length
// Ref: https://github.com/noir-lang/noir/issues/6163, https://gist.github.com/jp4g/d5953faae9eadb2909357474f7901e58
fn main(preimage: [u8; 448], result: [u8; 32]) {
// Construct arrays of different lengths
let mut preimage_511 = [0; 511];
let mut preimage_512 = [0; 512]; // Next block
let mut preimage_575 = [0; 575];
let mut preimage_576 = [0; 576]; // Next block
for i in 0..preimage.len() {
preimage_511[i] = preimage[i];
preimage_512[i] = preimage[i];
preimage_575[i] = preimage[i];
preimage_576[i] = preimage[i];
}
let fixed_length_hash = std::hash::sha256::digest(preimage);
let var_full_length_hash = std::hash::sha256::sha256_var(preimage, preimage.len() as u64);
let var_length_hash_511 = std::hash::sha256::sha256_var(preimage_511, preimage.len() as u64);
let var_length_hash_512 = std::hash::sha256::sha256_var(preimage_512, preimage.len() as u64);
let var_length_hash_575 = std::hash::sha256::sha256_var(preimage_575, preimage.len() as u64);
let var_length_hash_576 = std::hash::sha256::sha256_var(preimage_576, preimage.len() as u64);

// All of the above should have produced the same hash (result)
assert(fixed_length_hash == result);
assert(var_full_length_hash == result);
assert(var_length_hash_511 == result);
assert(var_length_hash_512 == result);
assert(var_length_hash_575 == result);
assert(var_length_hash_576 == result);
}

0 comments on commit 76eec71

Please sign in to comment.