Skip to content

Commit

Permalink
const-oid: fix large arc handling (#1592)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tarcieri authored Nov 1, 2024
1 parent 2492851 commit 3ff4426
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 5 deletions.
3 changes: 1 addition & 2 deletions const-oid/src/arcs.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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::<Arc>();
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
Expand Down
10 changes: 7 additions & 3 deletions const-oid/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 @ ..] => {
Expand Down
23 changes: 23 additions & 0 deletions const-oid/tests/oid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,38 +34,49 @@ 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()
}

#[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);
assert_eq!(oid_largearc0.arc(2).unwrap(), 16384);
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);
Expand All @@ -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));
}
Expand Down

0 comments on commit 3ff4426

Please sign in to comment.