Skip to content

Commit 8a8498f

Browse files
committed
consensus: Add CompactSize range check to deserialization
Manually backport rust-bitcoin#5697. On `master` the `VarInt` type is gone but the deserialization bug fixed by 5697 exists here none the less. Add the same range check to `Decodable for VarInt`.
1 parent 2f39664 commit 8a8498f

2 files changed

Lines changed: 36 additions & 5 deletions

File tree

bitcoin/src/consensus/encode.rs

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ pub enum Error {
5757
},
5858
/// VarInt was encoded in a non-minimal way.
5959
NonMinimalVarInt,
60+
/// VarInt value exceeds the maximum allowed size.
61+
OversizedVarInt,
6062
/// Parsing error.
6163
ParseFailed(&'static str),
6264
/// Unsupported Segwit flag.
@@ -76,6 +78,7 @@ impl fmt::Display for Error {
7678
InvalidChecksum { expected: ref e, actual: ref a } =>
7779
write!(f, "invalid checksum: expected {:x}, actual {:x}", e.as_hex(), a.as_hex()),
7880
NonMinimalVarInt => write!(f, "non-minimal varint"),
81+
Self::OversizedVarInt => write!(f, "value exceeds the maximum allowed varint"),
7982
ParseFailed(ref s) => write!(f, "parse failed: {}", s),
8083
UnsupportedSegwitFlag(ref swflag) =>
8184
write!(f, "unsupported segwit version: {}", swflag),
@@ -93,6 +96,7 @@ impl std::error::Error for Error {
9396
OversizedVectorAllocation { .. }
9497
| InvalidChecksum { .. }
9598
| NonMinimalVarInt
99+
| OversizedVarInt
96100
| ParseFailed(_)
97101
| UnsupportedSegwitFlag(_) => None,
98102
}
@@ -494,7 +498,7 @@ impl Decodable for VarInt {
494498
#[inline]
495499
fn consensus_decode<R: Read + ?Sized>(r: &mut R) -> Result<Self, Error> {
496500
let n = ReadExt::read_u8(r)?;
497-
match n {
501+
let varint = match n {
498502
0xFF => {
499503
let x = ReadExt::read_u64(r)?;
500504
if x < 0x100000000 {
@@ -520,10 +524,25 @@ impl Decodable for VarInt {
520524
}
521525
}
522526
n => Ok(VarInt::from(n)),
527+
};
528+
match varint {
529+
Ok(v) => {
530+
if v.0 > MAX_COMPACT_SIZE as u64 {
531+
Err(Error::OversizedVarInt.into())
532+
} else {
533+
Ok(v)
534+
}
535+
}
536+
Err(e) => Err(e),
523537
}
524538
}
525539
}
526540

541+
/// The maximum size of a serialized object in bytes or number of elements
542+
/// (for eg vectors) when the size is encoded as CompactSize.
543+
/// <https://github.com/bitcoin/bitcoin/blob/a7c29df0e5ace05b6186612671d6103c112ec922/src/serialize.h#L32>
544+
pub const MAX_COMPACT_SIZE: usize = 0x0200_0000;
545+
527546
impl Encodable for bool {
528547
#[inline]
529548
fn consensus_encode<W: Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
@@ -962,10 +981,6 @@ mod tests {
962981
serialize(&VarInt(0xF0F0F0F0F0E0)),
963982
vec![0xFFu8, 0xE0, 0xF0, 0xF0, 0xF0, 0xF0, 0xF0, 0, 0]
964983
);
965-
assert_eq!(
966-
test_varint_encode(0xFF, &0x100000000_u64.to_le_bytes()).unwrap(),
967-
VarInt(0x100000000)
968-
);
969984
assert_eq!(test_varint_encode(0xFE, &0x10000_u64.to_le_bytes()).unwrap(), VarInt(0x10000));
970985
assert_eq!(test_varint_encode(0xFD, &0xFD_u64.to_le_bytes()).unwrap(), VarInt(0xFD));
971986

@@ -980,6 +995,20 @@ mod tests {
980995
test_varint_len(VarInt(u64::MAX), 9);
981996
}
982997

998+
#[test]
999+
fn deserialize_varint_too_large() {
1000+
// MAX_COMPACT_SIZE (0x02000000) should succeed
1001+
assert_eq!(test_varint_encode(0xFE, &(0x02000000_u64).to_le_bytes()).unwrap(), VarInt(0x02000000));
1002+
// MAX_COMPACT_SIZE + 1 should fail with range check enabled
1003+
let mut input = [0u8; 9];
1004+
input[0] = 0xFE;
1005+
input[1..5].copy_from_slice(&(0x02000001_u32).to_le_bytes());
1006+
assert_eq!(
1007+
discriminant(&deserialize_partial::<VarInt>(&input).map(|t| t.0).unwrap_err()),
1008+
discriminant(&Error::OversizedVarInt)
1009+
);
1010+
}
1011+
9831012
fn test_varint_len(varint: VarInt, expected: usize) {
9841013
let mut encoder = vec![];
9851014
assert_eq!(varint.consensus_encode(&mut encoder).unwrap(), expected);

bitcoin/src/consensus/serde.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,8 @@ fn consensus_error_into_serde<E: serde::de::Error>(error: ConsensusError) -> E {
368368
),
369369
ConsensusError::NonMinimalVarInt =>
370370
E::custom(format_args!("compact size was not encoded minimally")),
371+
ConsensusError::OversizedVarInt =>
372+
E::custom(format_args!("compact size was too big")),
371373
ConsensusError::ParseFailed(msg) => E::custom(msg),
372374
ConsensusError::UnsupportedSegwitFlag(flag) =>
373375
E::invalid_value(Unexpected::Unsigned(flag.into()), &"segwit version 1 flag"),

0 commit comments

Comments
 (0)