From 8a64254c9520d7b954c852787966f2eb8da97180 Mon Sep 17 00:00:00 2001 From: Flakebi Date: Mon, 24 Dec 2018 00:23:52 +0100 Subject: [PATCH 1/4] Return error on invalid length --- src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index cffec33..7e17239 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -374,6 +374,9 @@ fn from_der_(i: &[u8], start_offset: usize) let soff = start_offset + index; let (tag, constructed, class) = decode_tag(i, &mut index); let len = decode_length(i, &mut index)?; + if i.len() < index + len { + return Err(ASN1DecodeErr::LengthTooLarge(index + len)); + } let body = &i[index .. (index + len)]; if class != ASN1Class::Universal { From 115859668c6e26d7bc231db4ed72237b36aa25db Mon Sep 17 00:00:00 2001 From: Flakebi Date: Fri, 29 Mar 2019 16:38:56 +0100 Subject: [PATCH 2/4] Remove usage of deprecated functions --- src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 7e17239..092a7d5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -821,8 +821,8 @@ pub fn to_der(i: &ASN1Block) -> Result,ASN1EncodeErr> { } &ASN1Block::GeneralizedTime(_, ref time) => { let base = time.format("%Y%m%d%H%M%S.%f").to_string(); - let zclear = base.trim_right_matches('0'); - let dclear = zclear.trim_right_matches('.'); + let zclear = base.trim_end_matches('0'); + let dclear = zclear.trim_end_matches('.'); let mut body = format!("{}Z", dclear).into_bytes(); let inttag = BigUint::from_u8(0x18).unwrap(); From 34764ca9bf2eaf89dfefc63ff01efa1f47c85025 Mon Sep 17 00:00:00 2001 From: Flakebi Date: Fri, 29 Mar 2019 17:51:08 +0100 Subject: [PATCH 3/4] Add cargo-fuzz target --- fuzz/.gitignore | 3 +++ fuzz/Cargo.toml | 22 ++++++++++++++++++++++ fuzz/fuzz_targets/fuzz_target_1.rs | 7 +++++++ 3 files changed, 32 insertions(+) create mode 100644 fuzz/.gitignore create mode 100644 fuzz/Cargo.toml create mode 100644 fuzz/fuzz_targets/fuzz_target_1.rs diff --git a/fuzz/.gitignore b/fuzz/.gitignore new file mode 100644 index 0000000..a092511 --- /dev/null +++ b/fuzz/.gitignore @@ -0,0 +1,3 @@ +target +corpus +artifacts diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml new file mode 100644 index 0000000..bd613a6 --- /dev/null +++ b/fuzz/Cargo.toml @@ -0,0 +1,22 @@ + +[package] +name = "simple_asn1-fuzz" +version = "0.0.1" +authors = ["Automatically generated"] +publish = false + +[package.metadata] +cargo-fuzz = true + +[dependencies.simple_asn1] +path = ".." +[dependencies.libfuzzer-sys] +git = "https://github.com/rust-fuzz/libfuzzer-sys.git" + +# Prevent this from interfering with workspaces +[workspace] +members = ["."] + +[[bin]] +name = "fuzz_target_1" +path = "fuzz_targets/fuzz_target_1.rs" diff --git a/fuzz/fuzz_targets/fuzz_target_1.rs b/fuzz/fuzz_targets/fuzz_target_1.rs new file mode 100644 index 0000000..2f2276b --- /dev/null +++ b/fuzz/fuzz_targets/fuzz_target_1.rs @@ -0,0 +1,7 @@ +#![no_main] +#[macro_use] extern crate libfuzzer_sys; +extern crate simple_asn1; + +fuzz_target!(|data: &[u8]| { + let _ = simple_asn1::from_der(data); +}); From 0641eff20bcbc2e9e6224dd81acac7f2c0b2c166 Mon Sep 17 00:00:00 2001 From: Flakebi Date: Fri, 29 Mar 2019 17:51:42 +0100 Subject: [PATCH 4/4] Add errors for panics found by fuzzing --- src/lib.rs | 117 ++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 88 insertions(+), 29 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 092a7d5..76a6820 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -273,7 +273,17 @@ pub enum ASN1DecodeErr { LengthTooLarge(usize), UTF8DecodeFailure(Utf8Error), PrintableStringDecodeFailure, - InvalidDateValue(String) + InvalidDateValue(String), + InvalidBitStringLength(isize), + /// Not a valid ASN.1 class + InvalidClass(u8), + /// Expected more input + /// + /// Invalid ASN.1 input can lead to this error. + Incomplete, + + #[doc(hidden)] + __Nonexhaustive, } impl fmt::Display for ASN1DecodeErr { @@ -290,7 +300,15 @@ impl fmt::Display for ASN1DecodeErr { ASN1DecodeErr::PrintableStringDecodeFailure => write!(f, "Printable string failed to properly decode."), ASN1DecodeErr::InvalidDateValue(x) => - write!(f, "Invalid date value: {}", x) + write!(f, "Invalid date value: {}", x), + ASN1DecodeErr::InvalidBitStringLength(i) => + write!(f, "Invalid length of bit string: {}", i), + ASN1DecodeErr::InvalidClass(i) => + write!(f, "Invalid class value: {}", i), + ASN1DecodeErr::Incomplete => + write!(f, "Incomplete data or invalid ASN1"), + ASN1DecodeErr::__Nonexhaustive => + panic!("A non exhaustive error should not be constructed"), } } } @@ -309,7 +327,15 @@ impl Error for ASN1DecodeErr { ASN1DecodeErr::PrintableStringDecodeFailure => "Printable string failed to properly decode.", ASN1DecodeErr::InvalidDateValue(_) => - "Invalid date value." + "Invalid date value.", + ASN1DecodeErr::InvalidClass(_) => + "Invalid class value", + ASN1DecodeErr::InvalidBitStringLength(_) => + "Invalid length of bit string", + ASN1DecodeErr::Incomplete => + "Incomplete data or invalid ASN1", + ASN1DecodeErr::__Nonexhaustive => + panic!("A non exhaustive error should not be constructed"), } } @@ -372,10 +398,11 @@ fn from_der_(i: &[u8], start_offset: usize) while index < len { let soff = start_offset + index; - let (tag, constructed, class) = decode_tag(i, &mut index); + let (tag, constructed, class) = decode_tag(i, &mut index)?; let len = decode_length(i, &mut index)?; - if i.len() < index + len { - return Err(ASN1DecodeErr::LengthTooLarge(index + len)); + let checklen = index.checked_add(len).ok_or(ASN1DecodeErr::LengthTooLarge(len))?; + if checklen > i.len() { + return Err(ASN1DecodeErr::Incomplete); } let body = &i[index .. (index + len)]; @@ -419,7 +446,14 @@ fn from_der_(i: &[u8], start_offset: usize) } Some(0x03) => { let bits = (&body[1..]).to_vec(); - let nbits = (bits.len() * 8) - (body[0] as usize); + let bitcount = bits.len() * 8; + let rest = body[0] as usize; + if bitcount < rest { + return Err(ASN1DecodeErr::InvalidBitStringLength( + bitcount as isize - rest as isize)); + } + + let nbits = bitcount - (body[0] as usize); result.push(ASN1Block::BitString(soff, nbits, bits)) } // OCTET STRING @@ -433,6 +467,9 @@ fn from_der_(i: &[u8], start_offset: usize) // OBJECT IDENTIFIER Some(0x06) => { let mut value1 = BigUint::zero(); + if body.len() == 0 { + return Err(ASN1DecodeErr::Incomplete) ; + } let mut value2 = BigUint::from_u8(body[0]).unwrap(); let mut oidres = Vec::new(); let mut bindex = 1; @@ -450,7 +487,7 @@ fn from_der_(i: &[u8], start_offset: usize) oidres.push(value1); oidres.push(value2); while bindex < body.len() { - oidres.push(decode_base127(body, &mut bindex)); + oidres.push(decode_base127(body, &mut bindex)?); } let res = OID(oidres); @@ -533,10 +570,17 @@ fn from_der_(i: &[u8], start_offset: usize) return Err(ASN1DecodeErr::InvalidDateValue(format!("{}",body.len()))); } - let mut v = String::from_iter(body.iter().map(|x| *x as char)); + let mut v: String = String::from_utf8(body.to_vec()) + .map_err(|e| ASN1DecodeErr::UTF8DecodeFailure(e.utf8_error()))?; + // Make sure the string is ascii, otherwise we cannot insert + // chars at specific bytes. + if !v.is_ascii() { + return Err(ASN1DecodeErr::InvalidDateValue(v)); + } + // We need to add padding back to the string if it's not there. - if v.find('.').is_none() { - v.insert(15, '.') + if !v.contains('.') { + v.insert(14, '.') } while v.len() < 25 { let idx = v.len() - 1; @@ -584,46 +628,57 @@ fn from_der_(i: &[u8], start_offset: usize) } /// Returns the tag, if the type is constructed and the class. -fn decode_tag(i: &[u8], index: &mut usize) -> (BigUint, bool, ASN1Class) { +fn decode_tag(i: &[u8], index: &mut usize) -> Result<(BigUint, bool, ASN1Class),ASN1DecodeErr> { + if *index >= i.len() { + return Err(ASN1DecodeErr::Incomplete) ; + } let tagbyte = i[*index]; let constructed = (tagbyte & 0b0010_0000) != 0; - let class = decode_class(tagbyte); + let class = decode_class(tagbyte)?; let basetag = tagbyte & 0b1_1111; *index += 1; + if basetag == 0b1_1111 { - let res = decode_base127(i, index); - (res, constructed, class) + let res = decode_base127(i, index)?; + Ok((res, constructed, class)) } else { - (BigUint::from(basetag), constructed, class) + Ok((BigUint::from(basetag), constructed, class)) } } -fn decode_base127(i: &[u8], index: &mut usize) -> BigUint { +fn decode_base127(i: &[u8], index: &mut usize) -> Result { let mut res = BigUint::zero(); loop { + if *index >= i.len() { + return Err(ASN1DecodeErr::Incomplete) ; + } + let nextbyte = i[*index]; *index += 1; res = (res << 7) + BigUint::from(nextbyte & 0x7f); if (nextbyte & 0x80) == 0 { - return res; + return Ok(res); } } } -fn decode_class(i: u8) -> ASN1Class { +fn decode_class(i: u8) -> Result { match i >> 6 { - 0b00 => ASN1Class::Universal, - 0b01 => ASN1Class::Application, - 0b10 => ASN1Class::ContextSpecific, - 0b11 => ASN1Class::Private, - _ => panic!("The universe is broken.") + 0b00 => Ok(ASN1Class::Universal), + 0b01 => Ok(ASN1Class::Application), + 0b10 => Ok(ASN1Class::ContextSpecific), + 0b11 => Ok(ASN1Class::Private), + _ => Err(ASN1DecodeErr::InvalidClass(i)), } } fn decode_length(i: &[u8], index: &mut usize) -> Result { + if *index >= i.len() { + return Err(ASN1DecodeErr::Incomplete) ; + } let startbyte = i[*index]; // NOTE: Technically, this size can be much larger than a usize. @@ -640,6 +695,10 @@ fn decode_length(i: &[u8], index: &mut usize) -> Result { } while lenlen > 0 { + if *index >= i.len() { + return Err(ASN1DecodeErr::Incomplete) ; + } + res = (res << 8) + (i[*index] as usize); *index += 1; @@ -1071,11 +1130,11 @@ mod tests { quickcheck! { fn class_encdec_roundtrips(c: ASN1Class) -> bool { - c == decode_class(encode_class(c.clone())) + c == decode_class(encode_class(c.clone())).unwrap() } fn class_decenc_roundtrips(v: u8) -> bool { - (v & 0b11000000) == encode_class(decode_class(v)) + (v & 0b11000000) == encode_class(decode_class(v).unwrap()) } } @@ -1095,7 +1154,7 @@ mod tests { fn tags_encdec_roundtrips(c: ASN1Class, con: bool, t: RandomUint) -> bool { let bytes = encode_tag(c, con, &t.x); let mut zero = 0; - let (t2, con2, c2) = decode_tag(&bytes[..], &mut zero); + let (t2, con2, c2) = decode_tag(&bytes[..], &mut zero).unwrap(); (c == c2) && (con == con2) && (t.x == t2) } @@ -1417,7 +1476,7 @@ mod tests { #[test] fn x509_tests() { - assert!(can_parse("test/server.bin").is_ok()); - assert!(can_parse("test/key.bin").is_ok()); + can_parse("test/server.bin").unwrap(); + can_parse("test/key.bin").unwrap(); } }