Skip to content

Commit

Permalink
Stop unsafe preallocation during JoinSplit deserialization
Browse files Browse the repository at this point in the history
Zebra believes the untrusted `JoinSplit` list size field when
deserializing `JoinSplit`s, and preallocates a `Vec` based on that size.

This is trivial a memory exhaustion attack.

Instead, use the current auto-growing implementation, which is limited
by the size of the message data.
  • Loading branch information
teor2345 committed Mar 19, 2021
1 parent c57baad commit e763ea3
Showing 1 changed file with 16 additions and 18 deletions.
34 changes: 16 additions & 18 deletions zebra-chain/src/transaction/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,22 @@ impl<P: ZkSnarkProof> ZcashSerialize for JoinSplitData<P> {

impl<P: ZkSnarkProof> ZcashDeserialize for Option<JoinSplitData<P>> {
fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
let num_joinsplits = reader.read_compactsize()?;
match num_joinsplits {
0 => Ok(None),
n => {
let first = sprout::JoinSplit::zcash_deserialize(&mut reader)?;
let mut rest = Vec::with_capacity((n - 1) as usize);
for _ in 0..(n - 1) {
rest.push(sprout::JoinSplit::zcash_deserialize(&mut reader)?);
}
let pub_key = reader.read_32_bytes()?.into();
let sig = reader.read_64_bytes()?.into();
Ok(Some(JoinSplitData {
first,
rest,
pub_key,
sig,
}))
}
let joinsplits: Vec<sprout::JoinSplit<P>> = Vec::zcash_deserialize(&mut reader)?;

if joinsplits.is_empty() {
Ok(None)
} else {
let (first, rest) = joinsplits
.split_first()
.expect("a non-empty Vec must have at least one entry");
let pub_key = reader.read_32_bytes()?.into();
let sig = reader.read_64_bytes()?.into();
Ok(Some(JoinSplitData {
first: first.clone(),
rest: rest.to_vec(),
pub_key,
sig,
}))
}
}
}
Expand Down

0 comments on commit e763ea3

Please sign in to comment.