Skip to content

Negative test case for constructed, definite-length octet string #1931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Negative test case for constructed, definite-length octet string #1931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions der/src/asn1/any.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,7 @@ impl<'a> AnyRef<'a> {

/// Returns [`Tag`] and [`Length`] of self.
pub fn header(&self) -> Header {
Header {
tag: self.tag,
length: self.value.len(),
}
Header::new(self.tag, self.value.len())
}

/// Attempt to decode this [`AnyRef`] type into the inner value.
Expand Down Expand Up @@ -131,7 +128,7 @@ impl<'a> DecodeValue<'a> for AnyRef<'a> {

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self, Error> {
Ok(Self {
tag: header.tag,
tag: header.tag(),
value: <&'a BytesRef>::decode_value(reader, header)?,
})
}
Expand Down Expand Up @@ -210,10 +207,7 @@ mod allocating {

/// Returns [`Tag`] and [`Length`] of self.
pub fn header(&self) -> Header {
Header {
tag: self.tag,
length: self.value.len(),
}
Header::new(self.tag, self.value.len())
}

/// Attempt to decode this [`Any`] type into the inner value.
Expand Down Expand Up @@ -295,7 +289,7 @@ mod allocating {

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self, Error> {
Ok(Self {
tag: header.tag,
tag: header.tag(),
value: BytesOwned::decode_value(reader, header)?,
})
}
Expand Down
7 changes: 2 additions & 5 deletions der/src/asn1/bit_string.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,7 @@ impl<'a> DecodeValue<'a> for BitStringRef<'a> {
type Error = Error;

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
let header = Header {
tag: header.tag,
length: (header.length - Length::ONE)?,
};
let header = header.copy_with_length((header.length() - Length::ONE)?);

let unused_bits = reader.read_byte()?;
let inner = <&'a BytesRef>::decode_value(reader, header)?;
Expand Down Expand Up @@ -354,7 +351,7 @@ mod allocating {
type Error = Error;

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
let inner_len = (header.length - Length::ONE)?;
let inner_len = (header.length() - Length::ONE)?;
let unused_bits = reader.read_byte()?;
let inner = reader.read_vec(inner_len)?;
Self::new(unused_bits, inner)
Expand Down
2 changes: 1 addition & 1 deletion der/src/asn1/bmp_string.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl<'a> DecodeValue<'a> for BmpString {
type Error = Error;

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
Self::from_ucs2(reader.read_vec(header.length)?)
Self::from_ucs2(reader.read_vec(header.length())?)
}
}

Expand Down
2 changes: 1 addition & 1 deletion der/src/asn1/boolean.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ impl<'a> DecodeValue<'a> for bool {
type Error = Error;

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
if header.length != Length::ONE {
if header.length() != Length::ONE {
return Err(reader.error(ErrorKind::Length { tag: Self::TAG }));
}

Expand Down
2 changes: 1 addition & 1 deletion der/src/asn1/generalized_time.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl<'a> DecodeValue<'a> for GeneralizedTime {
type Error = Error;

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
if Self::LENGTH != usize::try_from(header.length)? {
if Self::LENGTH != usize::try_from(header.length())? {
return Err(reader.error(Self::TAG.value_error()));
}

Expand Down
8 changes: 4 additions & 4 deletions der/src/asn1/integer/int.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ macro_rules! impl_encoding_traits {

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> $crate::Result<Self> {
let mut buf = [0u8; Self::BITS as usize / 8];
let max_length = u32::from(header.length) as usize;
let max_length = u32::from(header.length()) as usize;

if max_length == 0 {
return Err(reader.error(Tag::Integer.length_error()));
Expand All @@ -39,7 +39,7 @@ macro_rules! impl_encoding_traits {
};

// Ensure we compute the same encoded length as the original any value
if header.length != result.value_len()? {
if header.length() != result.value_len()? {
return Err(reader.error(Self::TAG.non_canonical_error()));
}

Expand Down Expand Up @@ -143,7 +143,7 @@ impl<'a> DecodeValue<'a> for IntRef<'a> {
let result = Self::new(bytes.as_slice())?;

// Ensure we compute the same encoded length as the original any value.
if result.value_len()? != header.length {
if result.value_len()? != header.length() {
return Err(reader.error(Self::TAG.non_canonical_error()));
}

Expand Down Expand Up @@ -237,7 +237,7 @@ mod allocating {
let result = Self::new(bytes.as_slice())?;

// Ensure we compute the same encoded length as the original any value.
if result.value_len()? != header.length {
if result.value_len()? != header.length() {
return Err(reader.error(Self::TAG.non_canonical_error()));
}

Expand Down
8 changes: 4 additions & 4 deletions der/src/asn1/integer/uint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ macro_rules! impl_encoding_traits {
const UNSIGNED_HEADROOM: usize = 1;

let mut buf = [0u8; (Self::BITS as usize / 8) + UNSIGNED_HEADROOM];
let max_length = u32::from(header.length) as usize;
let max_length = u32::from(header.length()) as usize;

if max_length == 0 {
return Err(reader.error(Tag::Integer.length_error()));
Expand All @@ -36,7 +36,7 @@ macro_rules! impl_encoding_traits {
let result = Self::from_be_bytes(decode_to_array(bytes)?);

// Ensure we compute the same encoded length as the original any value
if header.length != result.value_len()? {
if header.length() != result.value_len()? {
return Err(reader.error(Self::TAG.non_canonical_error()));
}

Expand Down Expand Up @@ -126,7 +126,7 @@ impl<'a> DecodeValue<'a> for UintRef<'a> {
let result = Self::new(decode_to_slice(bytes)?)?;

// Ensure we compute the same encoded length as the original any value.
if result.value_len()? != header.length {
if result.value_len()? != header.length() {
return Err(reader.error(Self::TAG.non_canonical_error()));
}

Expand Down Expand Up @@ -221,7 +221,7 @@ mod allocating {
let result = Self::new(decode_to_slice(bytes.as_slice())?)?;

// Ensure we compute the same encoded length as the original any value.
if result.value_len()? != header.length {
if result.value_len()? != header.length() {
return Err(reader.error(Self::TAG.non_canonical_error()));
}

Expand Down
10 changes: 5 additions & 5 deletions der/src/asn1/internal_macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ macro_rules! impl_custom_class {
let header = Header::decode(reader)?;

// the encoding shall be constructed if the base encoding is constructed
if header.tag.is_constructed() != T::CONSTRUCTED
if header.tag().is_constructed() != T::CONSTRUCTED
&& reader.encoding_rules() == EncodingRules::Der {
return Err(reader.error(header.tag.non_canonical_error()).into());
return Err(reader.error(header.tag().non_canonical_error()).into());
}

// read_value checks if header matches decoded length
Expand Down Expand Up @@ -183,10 +183,10 @@ macro_rules! impl_custom_class {
let header = Header::decode(reader)?;

// encoding shall be constructed
if !header.tag.is_constructed() {
return Err(reader.error(header.tag.non_canonical_error()).into());
if !header.tag().is_constructed() {
return Err(reader.error(header.tag().non_canonical_error()).into());
}
match header.tag {
match header.tag() {
Tag::$class_enum_name { number, .. } => Ok(Self {
tag_number: number,
tag_mode: TagMode::default(),
Expand Down
2 changes: 1 addition & 1 deletion der/src/asn1/null.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ impl<'a> DecodeValue<'a> for Null {
type Error = Error;

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
if header.length.is_zero() {
if header.length().is_zero() {
Ok(Null)
} else {
Err(reader.error(ErrorKind::Length { tag: Self::TAG }))
Expand Down
42 changes: 41 additions & 1 deletion der/src/asn1/octet_string.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,25 @@ mod tests {

#[test]
#[cfg(feature = "alloc")]
fn decode_ber() {
fn decode_ber_primitive_definite() {
use crate::{Decode, asn1::OctetString};
use hex_literal::hex;

const EXAMPLE: &[u8] = &hex!(
"040c" // primitive definite length OCTET STRING
"48656c6c6f2c20776f726c64" // "Hello, world"
);

let decoded = OctetString::from_ber(EXAMPLE).unwrap();
assert_eq!(decoded.as_bytes(), b"Hello, world");

let decoded = OctetString::from_der(EXAMPLE).unwrap();
assert_eq!(decoded.as_bytes(), b"Hello, world");
}

#[test]
#[cfg(feature = "alloc")]
fn decode_ber_constructed_indefinite() {
use crate::{Decode, asn1::OctetString};
use hex_literal::hex;

Expand All @@ -390,6 +408,28 @@ mod tests {
assert_eq!(decoded.as_bytes(), b"Hello, world");
}

#[test]
#[cfg(feature = "alloc")]
fn decode_ber_constructed_definite() {
use crate::{Decode, Error, ErrorKind, Length, Tag, asn1::OctetString};
use hex_literal::hex;

const EXAMPLE_BER: &[u8] = &hex!(
"2410" // Constructed definite length OCTET STRING
"040648656c6c6f2c" // Segment containing "Hello,"
"040620776f726c64" // Segment containing " world"
);

let err = OctetString::from_ber(EXAMPLE_BER).err().unwrap();
let expected = Error::new(
ErrorKind::Noncanonical {
tag: Tag::OctetString,
},
Length::new(1),
);
assert_eq!(expected, err);
}

#[test]
#[cfg(feature = "alloc")]
fn decode_context_specific_ber_explicit() {
Expand Down
4 changes: 2 additions & 2 deletions der/src/asn1/oid.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ impl<'a, const MAX_SIZE: usize> DecodeValue<'a> for ObjectIdentifier<MAX_SIZE> {
fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
let mut buf = [0u8; MAX_SIZE];
let slice = buf
.get_mut(..header.length.try_into()?)
.get_mut(..header.length().try_into()?)
.ok_or_else(|| Self::TAG.length_error())?;

let actual_len = reader.read_into(slice)?.len();
debug_assert_eq!(actual_len, header.length.try_into()?);
debug_assert_eq!(actual_len, header.length().try_into()?);
Ok(ObjectIdentifierRef::from_bytes(slice)?.try_into()?)
}
}
Expand Down
2 changes: 1 addition & 1 deletion der/src/asn1/real.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl<'a> DecodeValue<'a> for f64 {
fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
let bytes = <&'a BytesRef>::decode_value(reader, header)?.as_slice();

if header.length == Length::ZERO {
if header.length() == Length::ZERO {
Ok(0.0)
} else if is_nth_bit_one::<7>(bytes) {
// Binary encoding from section 8.5.7 applies
Expand Down
2 changes: 1 addition & 1 deletion der/src/asn1/utc_time.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl<'a> DecodeValue<'a> for UtcTime {
type Error = Error;

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
if Self::LENGTH != usize::try_from(header.length)? {
if Self::LENGTH != usize::try_from(header.length())? {
return Err(reader.error(Self::TAG.value_error()));
}

Expand Down
2 changes: 1 addition & 1 deletion der/src/asn1/utf8_string.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl<'a> DecodeValue<'a> for String {
type Error = Error;

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
Ok(String::from_utf8(reader.read_vec(header.length)?)?)
Ok(String::from_utf8(reader.read_vec(header.length())?)?)
}
}

Expand Down
32 changes: 22 additions & 10 deletions der/src/bytes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl<'a> DecodeValue<'a> for &'a BytesRef {
type Error = Error;

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
BytesRef::new(reader.read_slice(header.length)?)
BytesRef::new(reader.read_slice(header.length())?)
}
}

Expand Down Expand Up @@ -118,8 +118,8 @@ impl<'a> arbitrary::Arbitrary<'a> for &'a BytesRef {
pub(crate) mod allocating {
use super::BytesRef;
use crate::{
DecodeValue, DerOrd, EncodeValue, EncodingRules, Error, Header, Length, Reader, Result,
Tag, Writer, length::indefinite::read_constructed_vec,
DecodeValue, DerOrd, EncodeValue, EncodingRules, Error, ErrorKind, Header, Length, Reader,
Result, Tag, Writer, length::indefinite::read_constructed_vec,
};
use alloc::{borrow::ToOwned, boxed::Box, vec::Vec};
use core::{borrow::Borrow, cmp::Ordering, ops::Deref};
Expand Down Expand Up @@ -151,12 +151,24 @@ pub(crate) mod allocating {
header: Header,
inner_tag: Tag,
) -> Result<Self> {
// Reassemble indefinite length string types
if reader.encoding_rules() == EncodingRules::Ber
&& header.length.is_indefinite()
&& !inner_tag.is_constructed()
{
return Self::new(read_constructed_vec(reader, header.length, inner_tag)?);
if header.is_constructed() {
if header.length().is_indefinite() && reader.encoding_rules() == EncodingRules::Ber
{
// Reassemble indefinite length string types
return Self::new(read_constructed_vec(reader, header.length(), inner_tag)?);
} else {
// NOTE:
// constructed strings with definite length unsupported
// See discussion
// - https://github.com/RustCrypto/formats/issues/779#issuecomment-3049869721
//
// NOTE: this repositions the error to be at the end of the header
// rather than at the beginning of the value
return Err(Error::new(
ErrorKind::Noncanonical { tag: header.tag() },
reader.position().saturating_sub(Length::ONE),
));
}
}

Self::decode_value(reader, header)
Expand Down Expand Up @@ -199,7 +211,7 @@ pub(crate) mod allocating {
type Error = Error;

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
reader.read_vec(header.length).and_then(Self::new)
reader.read_vec(header.length()).and_then(Self::new)
}
}

Expand Down
2 changes: 1 addition & 1 deletion der/src/decode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ where

fn decode<R: Reader<'a>>(reader: &mut R) -> Result<T, <T as DecodeValue<'a>>::Error> {
let header = Header::decode(reader)?;
header.tag.assert_eq(T::TAG)?;
header.tag().assert_eq(T::TAG)?;
read_value(reader, header, T::decode_value)
}
}
Expand Down
6 changes: 3 additions & 3 deletions der/src/document.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl<'a> Decode<'a> for Document {

fn decode<R: Reader<'a>>(reader: &mut R) -> Result<Document, Error> {
let header = Header::peek(reader)?;
let length = (header.encoded_len()? + header.length)?;
let length = (header.encoded_len()? + header.length())?;
let bytes = reader.read_slice(length)?;

Ok(Self {
Expand Down Expand Up @@ -324,9 +324,9 @@ impl ZeroizeOnDrop for SecretDocument {}
/// entire sequence including the header.
fn decode_sequence<'a>(decoder: &mut SliceReader<'a>) -> Result<&'a [u8], Error> {
let header = Header::peek(decoder)?;
header.tag.assert_eq(Tag::Sequence)?;
header.tag().assert_eq(Tag::Sequence)?;

let len = (header.encoded_len()? + header.length)?;
let len = (header.encoded_len()? + header.length())?;
decoder.read_slice(len)
}

Expand Down
2 changes: 1 addition & 1 deletion der/src/encode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ pub trait EncodeValue {
where
Self: Tagged,
{
Header::new(self.tag(), self.value_len()?)
Ok(Header::new(self.tag(), self.value_len()?))
}

/// Compute the length of this value (sans [`Tag`]+[`Length`] header) when
Expand Down
Loading
Loading