Skip to content

Commit ed80e36

Browse files
youdie006XAMPPRocky
authored andcommitted
fix(oid): reject OIDs whose second arc exceeds 39 under first arc 0/1
is_valid_oid only checked slice[0] <= MAX_OID_FIRST_OCTET (first arc <= 2). It never enforced the X.660/X.690 rule that when the first arc is 0 or 1 the second arc must be <= 39. Because the integer-packed codecs encode the first two arcs as first*40 + second, a larger second arc silently aliases onto a different OID: ObjectIdentifier::new(vec![0, 40]) was accepted, encoded, and decoded back as 1.0 (and 0.999 -> 2.919) - an OID-confusion issue where two distinct constructed values collapse to the same encoding. Reject the invalid second arc at construction, mirroring the existing first-arc guard (const-fn safe). MAX_OID_SECOND_OCTET already existed and was used in the codec math; this reuses it in the validator. First arc 2 keeps its unbounded second arc; single-arc OIDs still accepted; new_unchecked left untouched. Tightening construction revealed four XER round-trip fixtures that themselves used invalid OIDs (1.654.2.1, 1.8270.4.1); updated to valid equivalents (top-level arc 2, which has no second-arc bound) preserving multi-byte-arc coverage. Fixes #565
1 parent 3568553 commit ed80e36

2 files changed

Lines changed: 32 additions & 9 deletions

File tree

src/types/oid.rs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,18 @@ pub(crate) const MAX_OID_FIRST_OCTET: u32 = 2;
55
pub(crate) const MAX_OID_SECOND_OCTET: u32 = 39;
66

77
const fn is_valid_oid(slice: &[u32]) -> bool {
8-
!slice.is_empty() && slice[0] <= MAX_OID_FIRST_OCTET
8+
if slice.is_empty() || slice[0] > MAX_OID_FIRST_OCTET {
9+
return false;
10+
}
11+
// Per X.660/X.690, when the first arc is 0 or 1 the second arc must be
12+
// <= 39: the encoding packs the first two arcs as `first * 40 + second`,
13+
// so a larger second arc aliases onto a different OID on encode (e.g.
14+
// `0.40` would encode and decode back as `1.0`). Reject such values at
15+
// construction. The first arc `2` has no such upper bound on the second.
16+
if slice[0] < MAX_OID_FIRST_OCTET && slice.len() >= 2 && slice[1] > MAX_OID_SECOND_OCTET {
17+
return false;
18+
}
19+
true
920
}
1021

1122
/// A reference to a global unique identifier that identifies an concept, such
@@ -801,4 +812,16 @@ mod test {
801812
ObjectIdentifier::new(vec![1, 2]).unwrap()
802813
);
803814
}
815+
816+
#[test]
817+
fn second_arc_validation() {
818+
// First arc 2 does not bound the second arc.
819+
assert!(ObjectIdentifier::new(vec![2, 999]).is_some());
820+
// First arc 0 or 1: second arc up to 39 is valid (boundary).
821+
assert!(ObjectIdentifier::new(vec![0, 39]).is_some());
822+
// First arc 0 or 1: second arc above 39 would alias onto another OID
823+
// when encoded (e.g. 0.40 -> 1.0), so it must be rejected.
824+
assert!(ObjectIdentifier::new(vec![0, 40]).is_none());
825+
assert!(ObjectIdentifier::new(vec![1, 40]).is_none());
826+
}
804827
}

src/xer.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -288,9 +288,9 @@ mod tests {
288288
round_trip!(
289289
object_identifier,
290290
ObjectIdentifier,
291-
ObjectIdentifier::from(Oid::const_new(&[1, 654, 2, 1])),
291+
ObjectIdentifier::from(Oid::const_new(&[2, 654, 2, 1])),
292292
"OBJECT_IDENTIFIER",
293-
"1.654.2.1"
293+
"2.654.2.1"
294294
);
295295
round_trip!(
296296
sequence,
@@ -377,10 +377,10 @@ mod tests {
377377
inner: InnerTestA {
378378
hidden: Some(false),
379379
},
380-
oid: Some(ObjectIdentifier::from(Oid::const_new(&[1, 8270, 4, 1]))),
380+
oid: Some(ObjectIdentifier::from(Oid::const_new(&[2, 8270, 4, 1]))),
381381
},
382382
"NestedTestA",
383-
"<wine><true /></wine><grappa>00010203</grappa><inner><hidden><false /></hidden></inner><oid>1.8270.4.1</oid>"
383+
"<wine><true /></wine><grappa>00010203</grappa><inner><hidden><false /></hidden></inner><oid>2.8270.4.1</oid>"
384384
);
385385
round_trip!(
386386
sequence_with_defaults,
@@ -402,10 +402,10 @@ mod tests {
402402
wine: true,
403403
grappa: vec![0, 1, 2, 3].into(),
404404
inner: InnerTestA { hidden: None },
405-
oid: Some(ObjectIdentifier::from(Oid::const_new(&[1, 8270, 4, 1])))
405+
oid: Some(ObjectIdentifier::from(Oid::const_new(&[2, 8270, 4, 1])))
406406
},
407407
"NestedTestA",
408-
"<wine><true /></wine><grappa>00010203</grappa><inner /><oid>1.8270.4.1</oid>"
408+
"<wine><true /></wine><grappa>00010203</grappa><inner /><oid>2.8270.4.1</oid>"
409409
);
410410
round_trip!(
411411
extensible_sequence_without_extensions,
@@ -545,11 +545,11 @@ mod tests {
545545
inner: InnerTestA {
546546
hidden: Some(false),
547547
},
548-
oid: Some(ObjectIdentifier::from(Oid::const_new(&[1, 8270, 4, 1]))),
548+
oid: Some(ObjectIdentifier::from(Oid::const_new(&[2, 8270, 4, 1]))),
549549
}
550550
},
551551
"SequenceWithChoice",
552-
"<recursion><Leaf /></recursion><nested><wine><true /></wine><grappa>00010203</grappa><inner><hidden><false /></hidden></inner><oid>1.8270.4.1</oid></nested>"
552+
"<recursion><Leaf /></recursion><nested><wine><true /></wine><grappa>00010203</grappa><inner><hidden><false /></hidden></inner><oid>2.8270.4.1</oid></nested>"
553553
);
554554
round_trip!(
555555
sequence_with_element_after_sequence_of,

0 commit comments

Comments
 (0)