Skip to content

Commit

Permalink
fix: add check for unused deserialization data (#83)
Browse files Browse the repository at this point in the history
Adds an extra check for unused deserialization data to ensure proof
serialization is canonical. Removes redundant length checks.

Currently, serialized proof data is read as
[chunks](https://github.com/tari-project/bulletproofs-plus/blob/e01c380186111c1fbe3fba213d1b492beff11b9d/src/range_proof.rs#L1017-L1020)
and
[tuples](https://docs.rs/itertools/latest/itertools/trait.Itertools.html#method.tuples)
for parsing purposes. Each of these functions can have leftover data.
However, existing length checks ensure such conditions cannot occur.

This PR replaces the length checks with a final check that there is no
leftover data. This helps to assert that serialization is canonical and
simplifies deserialization.

Closes #82.
  • Loading branch information
AaronFeickert committed Oct 20, 2023
1 parent a81b105 commit 55191b8
Showing 1 changed file with 25 additions and 34 deletions.
59 changes: 25 additions & 34 deletions src/range_proof.rs
Expand Up @@ -78,8 +78,6 @@ const FIXED_PROOF_ELEMENTS: usize = 5;

/// Assorted serialization constants
const ENCODED_EXTENSION_SIZE: usize = 1;
const MIN_LI_LENGTH: usize = 1;
const MIN_RI_LENGTH: usize = 1;

/// # Example
/// ```
Expand Down Expand Up @@ -992,9 +990,9 @@ where
P: Compressable,
P::Compressed: FixedBytesRepr,
{
/// Serializes the proof into a byte array
/// Serializes the proof into a canonical byte array
/// The first byte is an encoding of the extension degree, which tells us the length of `d1`
/// Then we serialize the rest of the proof elements as 32-byte encodings
/// Then we serialize the rest of the proof elements as canonical byte encodings
pub fn to_bytes(&self) -> Vec<u8> {
// The total proof size: extension degree encoding, fixed elements, vectors
let mut buf = Vec::with_capacity(
Expand Down Expand Up @@ -1026,7 +1024,7 @@ where
buf
}

/// Deserializes the proof from a byte slice
/// Deserializes the proof from a canonical byte slice
/// First we parse the extension degree, validate it, and use it to parse `d1`
/// Then we parse the remainder of the proof elements, inferring the lengths of `li` and `ri`
pub fn from_bytes(slice: &[u8]) -> Result<Self, ProofError> {
Expand All @@ -1038,33 +1036,7 @@ where
as usize,
)?;

// Now ensure the proof is long enough to account for all required elements:
// - encoded extension degree
// - `d1`
// - fixed proof elements
// - `li`, `ri`
// Since `li` and `ri` have variable length that depends on parameters, we only require they be nonempty
if slice.len() <
ENCODED_EXTENSION_SIZE +
SERIALIZED_ELEMENT_SIZE *
((extension_degree as usize) + FIXED_PROOF_ELEMENTS + MIN_LI_LENGTH + MIN_RI_LENGTH)
{
return Err(ProofError::InvalidLength("Serialized proof is too short".to_string()));
}

// Also ensure that `li` and `ri` will have the same number of elements
if (slice.len() -
ENCODED_EXTENSION_SIZE -
SERIALIZED_ELEMENT_SIZE * ((extension_degree as usize) + FIXED_PROOF_ELEMENTS)) %
(SERIALIZED_ELEMENT_SIZE * 2) !=
0
{
return Err(ProofError::InvalidLength(
"Serialized proof has an invalid size".to_string(),
));
}

// The rest of the serialization is of 32-byte proof elements
// The rest of the serialization is of encoded proof elements
let mut chunks = slice
.get(ENCODED_EXTENSION_SIZE..)
.ok_or(ProofError::InvalidLength("Serialized proof is too short".to_string()))?
Expand Down Expand Up @@ -1134,8 +1106,12 @@ where
})?;

// Extract the inner-product folding vectors `li` and `ri`
let (li, ri) = chunks
.tuples()
let mut tuples = chunks.by_ref().tuples::<(&[u8], &[u8])>();
let (li, ri): (
Vec<<P as Compressable>::Compressed>,
Vec<<P as Compressable>::Compressed>,
) = tuples
.by_ref()
.map(|(l, r)| {
let bytes_l: [u8; SERIALIZED_ELEMENT_SIZE] = l
.try_into()
Expand All @@ -1152,6 +1128,21 @@ where
.into_iter()
.unzip();

// The inner-product folding vectors should not be empty
if li.is_empty() || ri.is_empty() {
return Err(ProofError::InvalidLength("Serialized proof is too short".to_string()));
}

// We want to ensure that no data remains unused, to ensure serialization is canonical
// To do so, we check two things:
// - the tuple iterator has no leftover data, meaning an extra proof element
// - the chunk iterator has no leftover data, meaning extra bytes that don't yield a full proof element
if tuples.into_buffer().len() > 0 || !chunks.remainder().is_empty() {
return Err(ProofError::InvalidLength(
"Unused data after deserialization".to_string(),
));
}

Ok(RangeProof {
a,
a1,
Expand Down

0 comments on commit 55191b8

Please sign in to comment.