Skip to content

Commit

Permalink
error: remove untrusted type API leak
Browse files Browse the repository at this point in the history
This commit replaces the `From<untrusted::EndOfInput>` impl on the
exported `Error` type with a crate-private `der::end_of_input_err`
helper. This prevents `untrusted` from leaking into the webpki API
surface.
  • Loading branch information
cpu committed Oct 18, 2023
1 parent 5d17ef0 commit 5390ec5
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 20 deletions.
2 changes: 1 addition & 1 deletion src/crl/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ impl<'a> IssuingDistributionPoint<'a> {
// to unwrap a Tag::Boolean constructed value.
fn decode_bool(value: untrusted::Input) -> Result<bool, Error> {
let mut reader = untrusted::Reader::new(value);
let value = reader.read_byte()?;
let value = reader.read_byte().map_err(der::end_of_input_err)?;
if !reader.at_end() {
return Err(Error::BadDer);
}
Expand Down
30 changes: 17 additions & 13 deletions src/der.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,47 +149,47 @@ pub(crate) fn read_tag_and_get_value_limited<'a>(
input: &mut untrusted::Reader<'a>,
size_limit: usize,
) -> Result<(u8, untrusted::Input<'a>), Error> {
let tag = input.read_byte()?;
let tag = input.read_byte().map_err(end_of_input_err)?;
if (tag & HIGH_TAG_RANGE_START) == HIGH_TAG_RANGE_START {
return Err(Error::BadDer); // High tag number form is not allowed.
}

// If the high order bit of the first byte is set to zero then the length
// is encoded in the seven remaining bits of that byte. Otherwise, those
// seven bits represent the number of bytes used to encode the length.
let length = match input.read_byte()? {
let length = match input.read_byte().map_err(end_of_input_err)? {
n if (n & SHORT_FORM_LEN_MAX) == 0 => usize::from(n),
LONG_FORM_LEN_ONE_BYTE => {
let length_byte = input.read_byte()?;
let length_byte = input.read_byte().map_err(end_of_input_err)?;
if length_byte < SHORT_FORM_LEN_MAX {
return Err(Error::BadDer); // Not the canonical encoding.
}
usize::from(length_byte)
}
LONG_FORM_LEN_TWO_BYTES => {
let length_byte_one = usize::from(input.read_byte()?);
let length_byte_two = usize::from(input.read_byte()?);
let length_byte_one = usize::from(input.read_byte().map_err(end_of_input_err)?);
let length_byte_two = usize::from(input.read_byte().map_err(end_of_input_err)?);
let combined = (length_byte_one << 8) | length_byte_two;
if combined <= LONG_FORM_LEN_ONE_BYTE_MAX {
return Err(Error::BadDer); // Not the canonical encoding.
}
combined
}
LONG_FORM_LEN_THREE_BYTES => {
let length_byte_one = usize::from(input.read_byte()?);
let length_byte_two = usize::from(input.read_byte()?);
let length_byte_three = usize::from(input.read_byte()?);
let length_byte_one = usize::from(input.read_byte().map_err(end_of_input_err)?);
let length_byte_two = usize::from(input.read_byte().map_err(end_of_input_err)?);
let length_byte_three = usize::from(input.read_byte().map_err(end_of_input_err)?);
let combined = (length_byte_one << 16) | (length_byte_two << 8) | length_byte_three;
if combined <= LONG_FORM_LEN_TWO_BYTES_MAX {
return Err(Error::BadDer); // Not the canonical encoding.
}
combined
}
LONG_FORM_LEN_FOUR_BYTES => {
let length_byte_one = usize::from(input.read_byte()?);
let length_byte_two = usize::from(input.read_byte()?);
let length_byte_three = usize::from(input.read_byte()?);
let length_byte_four = usize::from(input.read_byte()?);
let length_byte_one = usize::from(input.read_byte().map_err(end_of_input_err)?);
let length_byte_two = usize::from(input.read_byte().map_err(end_of_input_err)?);
let length_byte_three = usize::from(input.read_byte().map_err(end_of_input_err)?);
let length_byte_four = usize::from(input.read_byte().map_err(end_of_input_err)?);
let combined = (length_byte_one << 24)
| (length_byte_two << 16)
| (length_byte_three << 8)
Expand All @@ -208,7 +208,7 @@ pub(crate) fn read_tag_and_get_value_limited<'a>(
return Err(Error::BadDer); // The length is larger than the caller accepts.
}

let inner = input.read_bytes(length)?;
let inner = input.read_bytes(length).map_err(end_of_input_err)?;
Ok((tag, inner))
}

Expand Down Expand Up @@ -386,6 +386,10 @@ pub(crate) fn nonnegative_integer<'a>(
}
}

pub(crate) fn end_of_input_err(_: untrusted::EndOfInput) -> Error {
Error::BadDer
}

// Like mozilla::pkix, we accept the nonconformant explicit encoding of
// the default value (false) for compatibility with real-world certificates.
impl<'a> FromDer<'a> for bool {
Expand Down
6 changes: 0 additions & 6 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,6 @@ impl fmt::Display for Error {
#[cfg(feature = "std")]
impl ::std::error::Error for Error {}

impl From<untrusted::EndOfInput> for Error {
fn from(_: untrusted::EndOfInput) -> Self {
Error::BadDer
}
}

/// Trailing data was found while parsing DER-encoded input for the named type.
#[allow(missing_docs)]
#[non_exhaustive]
Expand Down

0 comments on commit 5390ec5

Please sign in to comment.