Skip to content
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.

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

Add fuzzer + fix issues found using it #10

Merged
merged 5 commits into from
Jan 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 17 additions & 17 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions picky-asn1-der/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Changelog

## [0.2.0] 2019-12-23

### Added

- Add `from_reader_with_max_len` deserialization function to limit how many bytes can be read at most.

### Changed

- `from_reader` function has a default limit of 10240 bytes before returning a truncated data error. Uses `from_reader_with_max_len` to change the limit.

### Fixed

- Fix various crash found by fuzzing.

2 changes: 1 addition & 1 deletion picky-asn1-der/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "picky-asn1-der"
version = "0.1.0"
version = "0.2.0"
edition = "2018"
authors = [
"KizzyCode Software Labs./Keziah Biermann <development@kizzycode.de>",
Expand Down
29 changes: 23 additions & 6 deletions picky-asn1-der/src/de/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ use picky_asn1::{tag::Tag, wrapper::*, Asn1Type};
use serde::{de::Visitor, Deserialize};
use std::io::{Cursor, Read};

const DEFAULT_MAX_LEN: usize = 10240;

/// Deserializes `T` from `bytes`
pub fn from_bytes<'a, T: Deserialize<'a>>(bytes: &'a [u8]) -> Result<T> {
debug_log!("deserialization using `from_bytes`");
Expand All @@ -22,8 +24,16 @@ pub fn from_bytes<'a, T: Deserialize<'a>>(bytes: &'a [u8]) -> Result<T> {

/// Deserializes `T` from `reader`
pub fn from_reader<'a, T: Deserialize<'a>>(reader: impl Read + 'a) -> Result<T> {
debug_log!("deserialization using `from_reader`");
let mut deserializer = Deserializer::new_from_reader(reader);
from_reader_with_max_len(reader, DEFAULT_MAX_LEN)
}

/// Deserializes `T` from `reader` reading at most n bytes.
pub fn from_reader_with_max_len<'a, T: Deserialize<'a>>(reader: impl Read + 'a, max_len: usize) -> Result<T> {
debug_log!(
"deserialization using `from_reader_with_max_len`, max_len = {}",
max_len
);
let mut deserializer = Deserializer::new_from_reader(reader, max_len);
T::deserialize(&mut deserializer)
}

Expand All @@ -33,20 +43,22 @@ pub struct Deserializer<'de> {
buf: Vec<u8>,
encapsulator_tag_stack: Vec<Tag>,
header_only: bool,
max_len: usize,
}

impl<'de> Deserializer<'de> {
/// Creates a new deserializer over `bytes`
pub fn new_from_bytes(bytes: &'de [u8]) -> Self {
Self::new_from_reader(Cursor::new(bytes))
Self::new_from_reader(Cursor::new(bytes), bytes.len())
}
/// Creates a new deserializer for `reader`
pub fn new_from_reader(reader: impl Read + 'de) -> Self {
pub fn new_from_reader(reader: impl Read + 'de, max_len: usize) -> Self {
Self {
reader: PeekableReader::new(Box::new(reader)),
buf: Vec::new(),
encapsulator_tag_stack: Vec::with_capacity(3),
header_only: false,
max_len,
}
}

Expand Down Expand Up @@ -74,8 +86,13 @@ impl<'de> Deserializer<'de> {
(tag, len)
};

if len > self.max_len {
debug_log!("TRUNCATED DATA (invalid len: found {}, max is {})", len, self.max_len);
return Err(Asn1DerError::TruncatedData);
}

self.buf.resize(len, 0);
self.reader.read_exact(&mut self.buf)?;
self.reader.read_exact(self.buf.as_mut_slice())?;

Ok(tag)
}
Expand Down Expand Up @@ -122,7 +139,7 @@ impl<'de> Deserializer<'de> {
};
}

if peeked.len() < cursor {
if peeked.len() <= cursor {
debug_log!("peek_object: TRUNCATED DATA (couldn't read object tag)");
return Err(Asn1DerError::TruncatedData);
}
Expand Down
10 changes: 9 additions & 1 deletion picky-asn1-der/src/de/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ pub struct Sequence<'a, 'de> {
de: &'a mut Deserializer<'de>,
len: usize,
}

impl<'a, 'de> Sequence<'a, 'de> {
/// Creates a lazy deserializer that can walk through the sequence's sub-elements
pub fn deserialize_lazy(de: &'a mut Deserializer<'de>, len: usize) -> Self {
Self { de, len }
}
}

impl<'a, 'de> SeqAccess<'de> for Sequence<'a, 'de> {
type Error = Asn1DerError;

Expand All @@ -24,7 +26,13 @@ impl<'a, 'de> SeqAccess<'de> for Sequence<'a, 'de> {
// Deserialize the element
let pos = self.de.reader.pos();
let element = seed.deserialize(&mut *self.de)?;
self.len -= self.de.reader.pos() - pos;

let read = self.de.reader.pos() - pos;
if self.len < read {
debug_log!("TRUNCATED DATA (read more than necessary??)");
return Err(Asn1DerError::TruncatedData);
}
self.len -= read;

Ok(Some(element))
}
Expand Down
2 changes: 1 addition & 1 deletion picky-asn1-der/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ mod misc;
mod ser;

pub use crate::{
de::{from_bytes, from_reader, Deserializer},
de::{from_bytes, from_reader, from_reader_with_max_len, Deserializer},
ser::{to_byte_buf, to_bytes, to_vec, to_writer, Serializer},
};

Expand Down
6 changes: 6 additions & 0 deletions picky-asn1-der/src/ser/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub struct Sequence<'a, 'se> {
buf: Cursor<Vec<u8>>,
tag: Tag,
}

impl<'a, 'se> Sequence<'a, 'se> {
/// Creates a lazy serializer that will serialize the sequence's sub-elements to `writer`
pub fn serialize_lazy(ser: &'a mut Serializer<'se>, tag: Tag) -> Self {
Expand All @@ -28,6 +29,7 @@ impl<'a, 'se> Sequence<'a, 'se> {
to_writer(value, &mut self.buf)?;
Ok(())
}

/// Finalizes the sequence
fn finalize(self) -> Result<usize> {
// Reclaim buffer
Expand All @@ -39,6 +41,7 @@ impl<'a, 'se> Sequence<'a, 'se> {
Ok(written)
}
}

impl<'a, 'se> serde::ser::SerializeSeq for Sequence<'a, 'se> {
type Ok = usize;
type Error = Asn1DerError;
Expand All @@ -50,6 +53,7 @@ impl<'a, 'se> serde::ser::SerializeSeq for Sequence<'a, 'se> {
self.finalize()
}
}

impl<'a, 'se> serde::ser::SerializeTuple for Sequence<'a, 'se> {
type Ok = usize;
type Error = Asn1DerError;
Expand All @@ -61,6 +65,7 @@ impl<'a, 'se> serde::ser::SerializeTuple for Sequence<'a, 'se> {
self.finalize()
}
}

impl<'a, 'se> serde::ser::SerializeStruct for Sequence<'a, 'se> {
type Ok = usize;
type Error = Asn1DerError;
Expand All @@ -72,6 +77,7 @@ impl<'a, 'se> serde::ser::SerializeStruct for Sequence<'a, 'se> {
self.finalize()
}
}

impl<'a, 'se> serde::ser::SerializeTupleStruct for Sequence<'a, 'se> {
type Ok = usize;
type Error = Asn1DerError;
Expand Down
4 changes: 2 additions & 2 deletions picky-server/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "picky-server"
version = "4.2.0"
version = "4.2.1"
authors = [
"jtrepanier-devolutions <jtrepanier@devolutions.net>",
"Benoît CORTIER <benoit.cortier@fried-world.eu>",
Expand All @@ -11,7 +11,7 @@ license = "MIT OR Apache-2.0"
repository = "https://github.com/Devolutions/picky-rs"

[dependencies]
picky = { version = "4.2", default-features = false, features = ["x509", "chrono_conversion"] }
picky = { version = "4.5", default-features = false, features = ["x509", "chrono_conversion"] }
picky-asn1 = "0.1"
mongodb = { package = "mongodb_cwal", version = "0.6", features = ["ssl"] }
curl = { git = "https://github.com/Devolutions/curl-rust", branch = "wayk" }
Expand Down
4 changes: 2 additions & 2 deletions picky-server/src/picky_controller.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use picky::{
key::{KeyError, OwnedPublicKey, PrivateKey},
key::{KeyError, PrivateKey, PublicKey},
oids,
pem::PemError,
signature::SignatureHashType,
Expand Down Expand Up @@ -83,7 +83,7 @@ impl Picky {

pub fn generate_intermediate(
intermediate_name: &str,
intermediate_key: OwnedPublicKey,
intermediate_key: PublicKey,
issuer_cert: &Cert,
issuer_key: &PrivateKey,
signature_hash_type: SignatureHashType,
Expand Down
4 changes: 2 additions & 2 deletions picky/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "picky"
version = "4.4.2"
version = "4.5.0"
authors = [
"jtrepanier-devolutions <jtrepanier@devolutions.net>",
"Benoît CORTIER <benoit.cortier@fried-world.eu>",
Expand All @@ -12,7 +12,7 @@ repository = "https://github.com/Devolutions/picky-rs"

[dependencies]
picky-asn1 = "0.1"
picky-asn1-der = "0.1"
picky-asn1-der = "0.2"
serde = { version = "1.0", features = ["derive"] }
oid = { version = "^0.1.1", features = ["serde_support"] }
base64 = "0.10"
Expand Down
4 changes: 4 additions & 0 deletions picky/fuzz/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
target
corpus
artifacts
Cargo.lock
23 changes: 23 additions & 0 deletions picky/fuzz/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@

[package]
name = "picky-fuzz"
version = "0.0.0"
authors = ["Automatically generated"]
publish = false
edition = "2018"

[package.metadata]
cargo-fuzz = true

[dependencies.picky]
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"
Loading