From 908a918e8b2392eccbb809f05805a67034de966b Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Fri, 7 Apr 2023 16:07:50 -0700 Subject: [PATCH] Fuzz to validate routing collision safety. The fuzzing target introduced in this commit attemps to assert "collision safety". Formally, this is the property that: matches(request, route) := request is matched to route collides(route1, route2) := there is a a collision between routes forall requests req. !exist routes r1, r2 s.t. matches(req, r1) AND matches(req, r2) AND not collides(r1, r2) Alternatively: forall requests req, routes r1, r2. matches(req, r1) AND matches(req, r2) => collides(r1, r2) The target was run for 20 CPU hours without failure. --- core/lib/fuzz/Cargo.toml | 13 ++ .../corpus/collision-matching/another.seed | 1 + .../fuzz/corpus/collision-matching/base.seed | 1 + .../corpus/collision-matching/complex.seed | 1 + .../fuzz/corpus/collision-matching/large.seed | 1 + core/lib/fuzz/targets/collision-matching.rs | 217 ++++++++++++++++++ core/lib/src/route/route.rs | 6 + core/lib/src/route/uri.rs | 4 +- core/lib/src/router/matcher.rs | 3 +- 9 files changed, 245 insertions(+), 2 deletions(-) create mode 100644 core/lib/fuzz/corpus/collision-matching/another.seed create mode 100644 core/lib/fuzz/corpus/collision-matching/base.seed create mode 100644 core/lib/fuzz/corpus/collision-matching/complex.seed create mode 100644 core/lib/fuzz/corpus/collision-matching/large.seed create mode 100644 core/lib/fuzz/targets/collision-matching.rs diff --git a/core/lib/fuzz/Cargo.toml b/core/lib/fuzz/Cargo.toml index 14a00c61..eeae9d2d 100644 --- a/core/lib/fuzz/Cargo.toml +++ b/core/lib/fuzz/Cargo.toml @@ -10,6 +10,13 @@ cargo-fuzz = true [dependencies] libfuzzer-sys = "0.4" +arbitrary = { version = "1.3", features = ["derive"] } + +[target.'cfg(afl)'.dependencies] +afl = "*" + +[target.'cfg(honggfuzz)'.dependencies] +honggfuzz = "*" [dependencies.rocket] path = ".." @@ -35,3 +42,9 @@ name = "uri-normalization" path = "targets/uri-normalization.rs" test = false doc = false + +[[bin]] +name = "collision-matching" +path = "targets/collision-matching.rs" +test = false +doc = false diff --git a/core/lib/fuzz/corpus/collision-matching/another.seed b/core/lib/fuzz/corpus/collision-matching/another.seed new file mode 100644 index 00000000..34add6a2 --- /dev/null +++ b/core/lib/fuzz/corpus/collision-matching/another.seed @@ -0,0 +1 @@ +01//foo/bar/b01/foo/a/b1/text/html diff --git a/core/lib/fuzz/corpus/collision-matching/base.seed b/core/lib/fuzz/corpus/collision-matching/base.seed new file mode 100644 index 00000000..0a8a0a38 --- /dev/null +++ b/core/lib/fuzz/corpus/collision-matching/base.seed @@ -0,0 +1 @@ +01//a/b01//a/b0/a/b diff --git a/core/lib/fuzz/corpus/collision-matching/complex.seed b/core/lib/fuzz/corpus/collision-matching/complex.seed new file mode 100644 index 00000000..2a69fa98 --- /dev/null +++ b/core/lib/fuzz/corpus/collision-matching/complex.seed @@ -0,0 +1 @@ +44/foo/bar/applicatiom/json1bazb01/foo/a/btext/plain1/fooktext/html diff --git a/core/lib/fuzz/corpus/collision-matching/large.seed b/core/lib/fuzz/corpus/collision-matching/large.seed new file mode 100644 index 00000000..374cb09e --- /dev/null +++ b/core/lib/fuzz/corpus/collision-matching/large.seed @@ -0,0 +1 @@ +------------------------------------------------------ diff --git a/core/lib/fuzz/targets/collision-matching.rs b/core/lib/fuzz/targets/collision-matching.rs new file mode 100644 index 00000000..b38a2875 --- /dev/null +++ b/core/lib/fuzz/targets/collision-matching.rs @@ -0,0 +1,217 @@ +#![cfg_attr(all(not(honggfuzz), not(afl)), no_main)] + +use arbitrary::{Arbitrary, Unstructured, Result, Error}; + +use rocket::http::QMediaType; +use rocket::local::blocking::{LocalRequest, Client}; +use rocket::http::{Method, Accept, ContentType, MediaType, uri::Origin}; +use rocket::route::{Route, RouteUri, dummy_handler}; + +#[derive(Arbitrary)] +struct ArbitraryRequestData<'a> { + method: ArbitraryMethod, + origin: ArbitraryOrigin<'a>, + format: Result, +} + +#[derive(Arbitrary)] +struct ArbitraryRouteData<'a> { + method: ArbitraryMethod, + uri: ArbitraryRouteUri<'a>, + format: Option, +} + +impl std::fmt::Debug for ArbitraryRouteData<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ArbitraryRouteData") + .field("method", &self.method.0) + .field("base", &self.uri.0.base()) + .field("path", &self.uri.0.unmounted_origin.to_string()) + .field("uri", &self.uri.0.uri.to_string()) + .field("format", &self.format.as_ref().map(|v| v.0.to_string())) + .finish() + } +} + +impl std::fmt::Debug for ArbitraryRequestData<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ArbitraryRequestData") + .field("method", &self.method.0) + .field("origin", &self.origin.0.to_string()) + .field("format", &self.format.as_ref() + .map_err(|v| v.0.to_string()) + .map(|v| v.0.to_string())) + .finish() + } +} + +impl<'c, 'a: 'c> ArbitraryRequestData<'a> { + fn into_local_request(self, client: &'c Client) -> LocalRequest<'c> { + let mut req = client.req(self.method.0, self.origin.0); + match self.format { + Ok(accept) => req.add_header(accept.0), + Err(content_type) => req.add_header(content_type.0), + } + + req + } +} + +impl<'a> ArbitraryRouteData<'a> { + fn into_route(self) -> Route { + let mut r = Route::ranked(0, self.method.0, self.uri.0.as_str(), dummy_handler); + r.format = self.format.map(|f| f.0); + r + } +} + +struct ArbitraryMethod(Method); + +struct ArbitraryOrigin<'a>(Origin<'a>); + +struct ArbitraryAccept(Accept); + +struct ArbitraryContentType(ContentType); + +struct ArbitraryMediaType(MediaType); + +struct ArbitraryRouteUri<'a>(RouteUri<'a>); + +impl<'a> Arbitrary<'a> for ArbitraryMethod { + fn arbitrary(u: &mut Unstructured<'a>) -> Result { + let all_methods = &[ + Method::Get, Method::Put, Method::Post, Method::Delete, Method::Options, + Method::Head, Method::Trace, Method::Connect, Method::Patch + ]; + + Ok(ArbitraryMethod(*u.choose(all_methods)?)) + } + + fn size_hint(_: usize) -> (usize, Option) { + (1, None) + } +} + +impl<'a> Arbitrary<'a> for ArbitraryOrigin<'a> { + fn arbitrary(u: &mut Unstructured<'a>) -> Result { + let string = u.arbitrary::<&str>()?; + if string.is_empty() { + return Err(Error::NotEnoughData); + } + + Origin::parse(string) + .map(ArbitraryOrigin) + .map_err(|_| Error::IncorrectFormat) + } + + fn size_hint(_: usize) -> (usize, Option) { + (1, None) + } +} + +impl<'a> Arbitrary<'a> for ArbitraryAccept { + fn arbitrary(u: &mut Unstructured<'a>) -> Result { + let media_type: ArbitraryMediaType = u.arbitrary()?; + Ok(Self(Accept::new(QMediaType(media_type.0, None)))) + } + + fn size_hint(depth: usize) -> (usize, Option) { + ArbitraryMediaType::size_hint(depth) + } +} + +impl<'a> Arbitrary<'a> for ArbitraryContentType { + fn arbitrary(u: &mut Unstructured<'a>) -> Result { + let media_type: ArbitraryMediaType = u.arbitrary()?; + Ok(ArbitraryContentType(ContentType(media_type.0))) + } + + fn size_hint(depth: usize) -> (usize, Option) { + ArbitraryMediaType::size_hint(depth) + } +} + +impl<'a> Arbitrary<'a> for ArbitraryMediaType { + fn arbitrary(u: &mut Unstructured<'a>) -> Result { + let known = [ + "txt", "html", "htm", "xml", "opf", "xhtml", "csv", "js", "css", "json", + "png", "gif", "bmp", "jpeg", "jpg", "webp", "avif", "svg", "ico", "flac", "wav", + "webm", "weba", "ogg", "ogv", "pdf", "ttf", "otf", "woff", "woff2", "mp3", "mp4", + "mpeg4", "wasm", "aac", "ics", "bin", "mpg", "mpeg", "tar", "gz", "tif", "tiff", "mov", + "zip", "cbz", "cbr", "rar", "epub", "md", "markdown" + ]; + + let choice = u.choose(&known[..])?; + let known = MediaType::from_extension(choice).unwrap(); + + let top = u.ratio(1, 100)?.then_some("*".into()).unwrap_or(known.top().to_string()); + let sub = u.ratio(1, 100)?.then_some("*".into()).unwrap_or(known.sub().to_string()); + let params = u.ratio(1, 10)? + .then_some(vec![]) + .unwrap_or(known.params().map(|(k, v)| (k.to_string(), v.to_owned())).collect()); + + let media_type = MediaType::new(top, sub).with_params(params); + Ok(ArbitraryMediaType(media_type)) + } + + fn size_hint(_: usize) -> (usize, Option) { + (3, None) + } +} + +impl<'a> Arbitrary<'a> for ArbitraryRouteUri<'a> { + fn arbitrary(u: &mut Unstructured<'a>) -> Result { + let (base, path) = (u.arbitrary::<&str>()?, u.arbitrary::<&str>()?); + if base.is_empty() || path.is_empty() { + return Err(Error::NotEnoughData); + } + + RouteUri::try_new(base, path) + .map(ArbitraryRouteUri) + .map_err(|_| Error::IncorrectFormat) + } + + fn size_hint(_: usize) -> (usize, Option) { + (2, None) + } +} + +type TestData<'a> = ( + ArbitraryRouteData<'a>, + ArbitraryRouteData<'a>, + ArbitraryRequestData<'a> +); + +fn fuzz((route_a, route_b, req): TestData<'_>) { + let rocket = rocket::custom(rocket::Config { + workers: 2, + log_level: rocket::log::LogLevel::Off, + cli_colors: false, + ..rocket::Config::debug_default() + }); + + let client = Client::untracked(rocket).expect("debug rocket is okay"); + let (route_a, route_b) = (route_a.into_route(), route_b.into_route()); + let local_request = req.into_local_request(&client); + let request = local_request.inner(); + + if route_a.matches(request) && route_b.matches(request) { + assert!(route_a.collides_with(&route_b)); + assert!(route_b.collides_with(&route_a)); + } +} + +#[cfg(all(not(honggfuzz), not(afl)))] +libfuzzer_sys::fuzz_target!(|data: TestData| { fuzz(data) }); + +#[cfg(honggbuzz)] +fn main() { + loop { + honggfuzz::fuzz!(|data: TestData| { fuzz(data) }); + } +} + +#[cfg(afl)] +fn main() { + afl::fuzz!(|data: TestData| { fuzz(data) }); +} diff --git a/core/lib/src/route/route.rs b/core/lib/src/route/route.rs index a069e806..994cb058 100644 --- a/core/lib/src/route/route.rs +++ b/core/lib/src/route/route.rs @@ -290,6 +290,12 @@ impl Route { self.uri = RouteUri::try_new(&base, &self.uri.unmounted_origin.to_string())?; Ok(self) } + + /// Returns `true` if `self` collides with `other`. + #[doc(hidden)] + pub fn collides_with(&self, other: &Route) -> bool { + crate::router::Collide::collides_with(self, other) + } } impl fmt::Display for Route { diff --git a/core/lib/src/route/uri.rs b/core/lib/src/route/uri.rs index 7d0f0f01..b3b8b19a 100644 --- a/core/lib/src/route/uri.rs +++ b/core/lib/src/route/uri.rs @@ -100,7 +100,9 @@ impl<'a> RouteUri<'a> { /// /// This is a fallible variant of [`RouteUri::new`] which returns an `Err` /// if `base` or `uri` cannot be parsed as [`Origin`]s. - pub(crate) fn try_new(base: &str, uri: &str) -> Result> { + /// INTERNAL! + #[doc(hidden)] + pub fn try_new(base: &str, uri: &str) -> Result> { let mut base = Origin::parse(base) .map_err(|e| e.into_owned())? .into_normalized_nontrailing() diff --git a/core/lib/src/router/matcher.rs b/core/lib/src/router/matcher.rs index 521d96c9..b6f34978 100644 --- a/core/lib/src/router/matcher.rs +++ b/core/lib/src/router/matcher.rs @@ -17,7 +17,8 @@ impl Route { /// * All static components in the route's query string are also in the /// request query string, though in any position. If there is no query /// in the route, requests with/without queries match. - pub(crate) fn matches(&self, req: &Request<'_>) -> bool { + #[doc(hidden)] + pub fn matches(&self, req: &Request<'_>) -> bool { self.method == req.method() && paths_match(self, req) && queries_match(self, req)