From b78533066db5b098f2420a2c7dba56e6fcb43179 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Thu, 24 Feb 2022 13:52:09 -0700 Subject: [PATCH] der: fix handling of oversized unsigned `INTEGER` inputs The previous implementation used `saturating_sub` rather than `checked_sub` to compute the number of leading zeroes to use, which would cause a panic if the input exceeded the output (see #446). This commit switches to `checked_sub`, returning `ErrorKind::Length` in the event the output buffer is too small for the given input. It also adds unit tests for this behavior as well as the happy paths. --- der/src/asn1/integer/uint.rs | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/der/src/asn1/integer/uint.rs b/der/src/asn1/integer/uint.rs index 75948d364..28fb7acef 100644 --- a/der/src/asn1/integer/uint.rs +++ b/der/src/asn1/integer/uint.rs @@ -28,9 +28,14 @@ pub(super) fn decode_to_slice(bytes: &[u8]) -> Result<&[u8]> { pub(super) fn decode_to_array(bytes: &[u8]) -> Result<[u8; N]> { let input = decode_to_slice(bytes)?; - // Input has leading zeroes removed, so we need to add them back + // Compute number of leading zeroes to add + let num_zeroes = N + .checked_sub(input.len()) + .ok_or_else(|| Tag::Integer.length_error())?; + + // Copy input into `N`-sized output buffer with leading zeroes let mut output = [0u8; N]; - output[N.saturating_sub(input.len())..].copy_from_slice(input); + output[num_zeroes..].copy_from_slice(input); Ok(output) } @@ -69,3 +74,27 @@ pub(super) fn strip_leading_zeroes(mut bytes: &[u8]) -> &[u8] { fn needs_leading_zero(bytes: &[u8]) -> bool { matches!(bytes.get(0), Some(byte) if *byte >= 0x80) } + +#[cfg(test)] +mod tests { + use super::decode_to_array; + use crate::{ErrorKind, Tag}; + + #[test] + fn decode_to_array_no_leading_zero() { + let arr = decode_to_array::<4>(&[1, 2]).unwrap(); + assert_eq!(arr, [0, 0, 1, 2]); + } + + #[test] + fn decode_to_array_leading_zero() { + let arr = decode_to_array::<4>(&[0x00, 0xFF, 0xFE]).unwrap(); + assert_eq!(arr, [0x00, 0x00, 0xFF, 0xFE]); + } + + #[test] + fn decode_to_array_oversized_input() { + let err = decode_to_array::<1>(&[1, 2, 3]).err().unwrap(); + assert_eq!(err.kind(), ErrorKind::Length { tag: Tag::Integer }); + } +}