Fix various decoding issues. (#74)

This PR fixes a number of decoding issues. Specifically:

   o  error out rather than panic when a nested value has a greater length
      than allowed by the outer value,
   o  check that there is enough data available before skipping over a
      primitive value’s content,
   o  checks that enough data is available before trying to parse a tag value,
   o  checks for correct encoding of bit strings: don’t allow the number of
      unused bits to be greater than 7 and that they are zero for an empty
      bit string,
   o  checks for correct encoding of object identifiers: they cannot be empty
      and the last byte must have bit 7 cleared.

The PR adds tests for these cases. It also adds a number of fuzzing targets
to be used with cargo fuzz.

This PR provides the fixes for CVE-2023-39914.

fuzz/.gitignore

@@ -0,0 +1,4 @@
+target
+corpus
+artifacts
+coverage

fuzz/Cargo.toml

@@ -0,0 +1,40 @@
+[package]
+name = "bcder-fuzz"
+version = "0.0.0"
+publish = false
+edition = "2018"
+
+[package.metadata]
+cargo-fuzz = true
+
+[dependencies]
+libfuzzer-sys = "0.4"
+
+[dependencies.bcder]
+path = ".."
+
+# Prevent this from interfering with workspaces
+[workspace]
+members = ["."]
+
+[profile.release]
+debug = 1
+
+[[bin]]
+name = "decode_oid"
+path = "fuzz_targets/decode_oid.rs"
+test = false
+doc = false
+
+[[bin]]
+name = "decode_int"
+path = "fuzz_targets/decode_int.rs"
+test = false
+doc = false
+
+
+[[bin]]
+name = "decode_strings"
+path = "fuzz_targets/decode_strings.rs"
+test = false
+doc = false

fuzz/fuzz_targets/decode_int.rs

@@ -0,0 +1,35 @@
+#![no_main]
+
+use libfuzzer_sys::fuzz_target;
+use bcder::decode::SliceSource;
+use bcder::int::{Integer, Unsigned};
+use bcder::{Mode, Tag};
+
+macro_rules! decode_builtin {
+    ( $data:expr, $fn:ident ) => {{
+        let _ = Mode::Ber.decode(SliceSource::new($data), |cons| {
+            cons.take_primitive_if(Tag::INTEGER, |prim| prim.$fn())
+        });
+    }}
+}
+
+fuzz_target!(|data: &[u8]| {
+    let _ = Mode::Ber.decode(SliceSource::new(data), |cons| {
+        Integer::take_from(cons)
+    });
+    let _ = Mode::Ber.decode(SliceSource::new(data), |cons| {
+        Unsigned::take_from(cons)
+    });
+
+    decode_builtin!(data, to_i8);
+    decode_builtin!(data, to_u8);
+    decode_builtin!(data, to_i16);
+    decode_builtin!(data, to_u16);
+    decode_builtin!(data, to_i32);
+    decode_builtin!(data, to_u32);
+    decode_builtin!(data, to_i64);
+    decode_builtin!(data, to_u64);
+    decode_builtin!(data, to_i128);
+    decode_builtin!(data, to_u128);
+});
+

fuzz/fuzz_targets/decode_oid.rs

@@ -0,0 +1,35 @@
+#![no_main]
+
+use libfuzzer_sys::fuzz_target;
+use bcder::decode::SliceSource;
+use bcder::{ConstOid, Oid};
+use bcder::Mode;
+
+pub const SHA256: ConstOid = Oid(&[96, 134, 72, 1, 101, 3, 4, 2, 1]);
+
+fuzz_target!(|data: &[u8]| {
+    let take = Mode::Ber.decode(SliceSource::new(data), |cons| {
+        Oid::take_from(cons)
+    });
+    let skip = Mode::Ber.decode(SliceSource::new(data), |cons| {
+        Oid::skip_in(cons)
+    }).is_ok();
+    assert_eq!(take.is_ok(), skip);
+
+    if let Ok(take) = take.as_ref() {
+        let _ = take.to_string();
+    }
+
+    let skip_if = Mode::Ber.decode(SliceSource::new(data), |cons| {
+        SHA256.skip_if(cons)
+    }).is_ok();
+
+    if skip_if {
+        assert_eq!(take.unwrap(), SHA256);
+    }
+
+    let _ = Mode::Ber.decode(SliceSource::new(data), |cons| {
+        Oid::skip_opt_in(cons)
+    });
+
+});

fuzz/fuzz_targets/decode_strings.rs

@@ -0,0 +1,53 @@
+#![no_main]
+
+use libfuzzer_sys::fuzz_target;
+use bcder::decode::SliceSource;
+use bcder::string::{
+    BitString, OctetString, Ia5String, NumericString, PrintableString,
+    Utf8String,
+};
+use bcder::Mode;
+
+macro_rules! decode_strings {
+    ( $data:expr, [ $( $mode:ident ),* ] ) => {{
+        $(
+            let take = Mode::$mode.decode(SliceSource::new($data), |cons| {
+                BitString::take_from(cons)
+            });
+            let skip = Mode::$mode.decode(SliceSource::new($data), |cons| {
+                BitString::skip_in(cons)
+            }).is_ok();
+            assert_eq!(take.is_ok(), skip);
+
+            if let Ok(take) = take {
+                assert!(take.unused() < 8);
+                assert!(take.octet_len() > 0 || take.unused() == 0);
+            }
+
+            let _ = Mode::$mode.decode(SliceSource::new($data), |cons| {
+                OctetString::take_from(cons)
+            });
+            let _ = Mode::$mode.decode(SliceSource::new($data), |cons| {
+                OctetString::take_opt_from(cons)
+            });
+            let _ = Mode::$mode.decode(SliceSource::new($data), |cons| {
+                Ia5String::take_from(cons)
+            });
+            let _ = Mode::$mode.decode(SliceSource::new($data), |cons| {
+                NumericString::take_from(cons)
+            });
+            let _ = Mode::$mode.decode(SliceSource::new($data), |cons| {
+                PrintableString::take_from(cons)
+            });
+            let _ = Mode::$mode.decode(SliceSource::new($data), |cons| {
+                Utf8String::take_from(cons)
+            });
+        )*
+    }}
+
+}
+
+fuzz_target!(|data: &[u8]| {
+    decode_strings!(data, [Ber, Cer, Der]);
+});
+

src/decode/content.rs

@@ -390,6 +390,25 @@ impl<'a, S: Source + 'a> Primitive<'a, S> {
         }
     }
 
+    /// Process a slice of the remainder of the content via a closure.
+    pub fn with_slice_all<F, T, E>(
+        &mut self, op: F,
+    ) -> Result<T, DecodeError<S::Error>>
+    where
+        F: FnOnce(&[u8]) -> Result<T, E>,
+        E: Into<ContentError>,
+    {
+        let remaining = self.remaining();
+        if self.source.request(remaining)? < remaining {
+            return Err(self.source.content_err("unexpected end of data"));
+        }
+        let res = op(&self.source.slice()[..remaining]).map_err(|err| {
+            self.content_err(err)
+        })?;
+        self.source.advance(remaining);
+        Ok(res)
+    }
+
     /// Checkes whether all content has been advanced over.
     fn exhausted(self) -> Result<(), DecodeError<S::Error>> {
         self.source.exhausted()
@@ -661,6 +680,13 @@ impl<'a, S: Source + 'a> Constructed<'a, S> {
 
         match length {
             Length::Definite(len) => {
+                if let Some(limit) = self.source.limit() {
+                    if len > limit {
+                        return Err(self.source.content_err(
+                            "nested value with excessive length"
+                        ))
+                    }
+                }
                 let old_limit = self.source.limit_further(Some(len));
                 let res = {
                     let mut content = if constructed {
@@ -1109,6 +1135,11 @@ impl<'a, S: Source + 'a> Constructed<'a, S> {
                         if let Err(err) = op(tag, constructed, stack.len()) {
                             return Err(self.content_err(err));
                         }
+                        if self.source.request(len)? < len {
+                            return Err(self.content_err(
+                                "short primitive value"
+                            ))
+                        }
                         self.source.advance(len);
                     }
                     else {

src/int.rs

@@ -78,15 +78,12 @@ macro_rules! slice_to_builtin {
 macro_rules! decode_builtin {
     ( $flavor:ident, $prim:expr, $type:ident) => {{
         Self::check_head($prim)?;
-        let res = {
-            let slice = $prim.slice_all()?;
+        $prim.with_slice_all(|slice| {
             slice_to_builtin!(
                 $flavor, slice, $type,
-                Err($prim.content_err("invalid integer"))
-            )?
-        };
-        $prim.skip_all()?;
-        Ok(res)
+                Err("invalid integer")
+            )
+        })
     }}
 }
 

src/length.rs

@@ -1,6 +1,6 @@
 //! The Length Octets.
 //!
-//! This is a private module. Its public tiems are re-exported by the parent.
+//! This is a private module. Its public items are re-exported by the parent.
 
 use std::io;
 use crate::decode::{DecodeError, Source};