From 3ff44265fd432100dd7e94aa728e8c2ce4d346b3 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Fri, 1 Nov 2024 13:07:31 -0600 Subject: [PATCH] const-oid: fix large arc handling (#1592) As originally reported in #1585 The maximum size of a serialized arc was miscomputed, since it failed to include the additional bits needed to store the LEB128 encoding. Also adds checked arithmetic to the calculation of an arc from base 10, both fixing a clippy warning and an overflow bug. --- const-oid/src/arcs.rs | 3 +-- const-oid/src/parser.rs | 10 +++++++--- const-oid/tests/oid.rs | 23 +++++++++++++++++++++++ 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/const-oid/src/arcs.rs b/const-oid/src/arcs.rs index 95cbbe0ef..20ebcb862 100644 --- a/const-oid/src/arcs.rs +++ b/const-oid/src/arcs.rs @@ -1,7 +1,6 @@ //! Arcs are integer values which exist within an OID's hierarchy. use crate::{Error, Result}; -use core::mem::size_of; #[cfg(doc)] use crate::ObjectIdentifier; @@ -25,7 +24,7 @@ pub(crate) const ARC_MAX_FIRST: Arc = 2; pub(crate) const ARC_MAX_SECOND: Arc = 39; /// Maximum number of bytes supported in an arc. -const ARC_MAX_BYTES: usize = size_of::(); +const ARC_MAX_BYTES: usize = (Arc::BITS as usize).div_ceil(7); /// Maximum value of the last byte in an arc. const ARC_MAX_LAST_OCTET: u8 = 0b11110000; // Max bytes of leading 1-bits diff --git a/const-oid/src/parser.rs b/const-oid/src/parser.rs index 41020f037..1df2335a6 100644 --- a/const-oid/src/parser.rs +++ b/const-oid/src/parser.rs @@ -49,11 +49,15 @@ impl Parser { } Err(err) => Err(err), }, - // TODO(tarcieri): checked arithmetic - #[allow(clippy::arithmetic_side_effects)] [byte @ b'0'..=b'9', remaining @ ..] => { let digit = byte.saturating_sub(b'0'); - self.current_arc = self.current_arc * 10 + digit as Arc; + self.current_arc = match self.current_arc.checked_mul(10) { + Some(arc) => match arc.checked_add(digit as Arc) { + Some(arc) => arc, + None => return Err(Error::ArcTooBig), + }, + None => return Err(Error::ArcTooBig), + }; self.parse_bytes(remaining) } [b'.', remaining @ ..] => { diff --git a/const-oid/tests/oid.rs b/const-oid/tests/oid.rs index bebbbadf5..50b89a59f 100644 --- a/const-oid/tests/oid.rs +++ b/const-oid/tests/oid.rs @@ -34,6 +34,12 @@ const EXAMPLE_OID_LARGE_ARC_1_BER: &[u8] = &hex!("0992268993F22C640101"); const EXAMPLE_OID_LARGE_ARC_1: ObjectIdentifier = ObjectIdentifier::new_unwrap(EXAMPLE_OID_LARGE_ARC_1_STR); +/// Example OID value with a large arc +const EXAMPLE_OID_LARGE_ARC_2_STR: &str = "1.2.840.10045.4.3.4.1111111111"; +const EXAMPLE_OID_LARGE_ARC_2_BER: &[u8] = &hex!("2A8648CE3D0403048491E8EB47"); +const EXAMPLE_OID_LARGE_ARC_2: ObjectIdentifier = + ObjectIdentifier::new_unwrap(crate::EXAMPLE_OID_LARGE_ARC_2_STR); + /// Create an OID from a string. pub fn oid(s: &str) -> ObjectIdentifier { ObjectIdentifier::new(s).unwrap() @@ -41,24 +47,28 @@ pub fn oid(s: &str) -> ObjectIdentifier { #[test] fn from_bytes() { + // 0.9.2342.19200300.100.1.1 let oid0 = ObjectIdentifier::from_bytes(EXAMPLE_OID_0_BER).unwrap(); assert_eq!(oid0.arc(0).unwrap(), 0); assert_eq!(oid0.arc(1).unwrap(), 9); assert_eq!(oid0.arc(2).unwrap(), 2342); assert_eq!(oid0, EXAMPLE_OID_0); + // 1.2.840.10045.2.1 let oid1 = ObjectIdentifier::from_bytes(EXAMPLE_OID_1_BER).unwrap(); assert_eq!(oid1.arc(0).unwrap(), 1); assert_eq!(oid1.arc(1).unwrap(), 2); assert_eq!(oid1.arc(2).unwrap(), 840); assert_eq!(oid1, EXAMPLE_OID_1); + // 2.16.840.1.101.3.4.1.42 let oid2 = ObjectIdentifier::from_bytes(EXAMPLE_OID_2_BER).unwrap(); assert_eq!(oid2.arc(0).unwrap(), 2); assert_eq!(oid2.arc(1).unwrap(), 16); assert_eq!(oid2.arc(2).unwrap(), 840); assert_eq!(oid2, EXAMPLE_OID_2); + // 1.2.16384 let oid_largearc0 = ObjectIdentifier::from_bytes(EXAMPLE_OID_LARGE_ARC_0_BER).unwrap(); assert_eq!(oid_largearc0.arc(0).unwrap(), 1); assert_eq!(oid_largearc0.arc(1).unwrap(), 2); @@ -66,6 +76,7 @@ fn from_bytes() { assert_eq!(oid_largearc0.arc(3), None); assert_eq!(oid_largearc0, EXAMPLE_OID_LARGE_ARC_0); + // 0.9.2342.19200300.100.1.1 let oid_largearc1 = ObjectIdentifier::from_bytes(EXAMPLE_OID_LARGE_ARC_1_BER).unwrap(); assert_eq!(oid_largearc1.arc(0).unwrap(), 0); assert_eq!(oid_largearc1.arc(1).unwrap(), 9); @@ -76,6 +87,18 @@ fn from_bytes() { assert_eq!(oid_largearc1.arc(6).unwrap(), 1); assert_eq!(oid_largearc1, EXAMPLE_OID_LARGE_ARC_1); + // 1.2.840.10045.4.3.4.1111111111 + let oid_largearc2 = ObjectIdentifier::from_bytes(EXAMPLE_OID_LARGE_ARC_2_BER).unwrap(); + assert_eq!(oid_largearc2.arc(0).unwrap(), 1); + assert_eq!(oid_largearc2.arc(1).unwrap(), 2); + assert_eq!(oid_largearc2.arc(2).unwrap(), 840); + assert_eq!(oid_largearc2.arc(3).unwrap(), 10045); + assert_eq!(oid_largearc2.arc(4).unwrap(), 4); + assert_eq!(oid_largearc2.arc(5).unwrap(), 3); + assert_eq!(oid_largearc2.arc(6).unwrap(), 4); + assert_eq!(oid_largearc2.arc(7).unwrap(), 1111111111); + assert_eq!(oid_largearc2, EXAMPLE_OID_LARGE_ARC_2); + // Empty assert_eq!(ObjectIdentifier::from_bytes(&[]), Err(Error::Empty)); }