diff --git a/src/asn1.rs b/src/asn1.rs index 7957b65..fb73447 100644 --- a/src/asn1.rs +++ b/src/asn1.rs @@ -231,9 +231,22 @@ impl<'a> AsnReader<'a> { self.read_i64_type(snmp::TYPE_TIMETICKS).map(|v| v as u32) } - #[allow(clippy::cast_sign_loss)] pub fn read_snmp_counter64(&mut self) -> Result { - self.read_i64_type(snmp::TYPE_COUNTER64).map(|v| v as u64) + self.read_u64_type(snmp::TYPE_COUNTER64) + } + + fn read_u64_type(&mut self, expected_ident: u8) -> Result { + let ident = self.read_byte()?; + if ident != expected_ident { + return Err(Error::AsnWrongType); + } + let val_len = self.read_length()?; + if val_len > self.inner.len() { + return Err(Error::AsnInvalidLen); + } + let (val, remaining) = self.inner.split_at(val_len); + self.inner = remaining; + decode_u64(val) } pub fn read_snmp_opaque(&mut self) -> Result<&'a [u8]> { @@ -253,6 +266,21 @@ impl<'a> AsnReader<'a> { } } +fn decode_u64(i: &[u8]) -> Result { + // Counter64 is an unsigned INTEGER (0..2^64-1). A value with bit 63 set is + // BER-encoded with a leading 0x00 sign octet and so occupies 9 octets, which + // the signed decoder rejects as an overflow. Accept up to 8 octets, or 9 + // octets when the extra leading octet is that 0x00 sign pad. + let bytes = match i { + [0x00, rest @ ..] if rest.len() == mem::size_of::() => rest, + _ if i.len() <= mem::size_of::() => i, + _ => return Err(Error::AsnIntOverflow), + }; + let mut buf = [0u8; 8]; + buf[(mem::size_of::() - bytes.len())..].copy_from_slice(bytes); + Ok(u64::from_be_bytes(buf)) +} + fn decode_i64(i: &[u8]) -> Result { if i.len() > mem::size_of::() { return Err(Error::AsnIntOverflow); diff --git a/src/tests.rs b/src/tests.rs index d2c630c..4fb02f5 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -371,3 +371,38 @@ fn test_v3_pdu_to_bytes() { assert_eq!(pdu2.req_id, 12345); assert_eq!(pdu2.version().unwrap(), Version::V3); } + +#[test] +fn read_counter64_large_values() { + // #36: a Counter64 >= 2^63 is BER-encoded as an unsigned INTEGER with a + // leading 0x00 sign octet and so occupies 9 octets. The old signed decoder + // rejected 9 octets as AsnIntOverflow, which silently dropped the varbind + // (and everything after it) from the response. + let cases: &[(&[u8], u64)] = &[ + (&[snmp::TYPE_COUNTER64, 0x01, 0x2a], 42), + ( + &[snmp::TYPE_COUNTER64, 0x09, 0x00, 0x80, 0, 0, 0, 0, 0, 0, 0], + 1u64 << 63, + ), + ( + &[ + snmp::TYPE_COUNTER64, + 0x09, + 0x00, + 0xff, + 0xff, + 0xff, + 0xff, + 0xff, + 0xff, + 0xff, + 0xff, + ], + u64::MAX, + ), + ]; + for (bytes, expected) in cases { + let mut reader = AsnReader::from_bytes(bytes); + assert_eq!(reader.read_snmp_counter64().unwrap(), *expected); + } +}