History

Assure we don't accidentally parse a valid-looking URL to `url` and cause long compute times.

The issue is that the `url` crate can be fooled by long URLs into parsing way-too-long hosts inefficiently enough to allow DoS attacks by slowing it down. Some predicate was implemented to detect such URLs before passing them to `url`, but unfortunately a somewhat valid looking URL could be crafted to bypass that check and still run into the `url` parser, which seemed to have no problem with the format of the URL. Testing showed that this is a very special case, which is now handled as well. We hope that one day `url` will handle this all by itself.

Sebastian Thiel committed 3 years ago

Commit 74975531 parent 82874cd3 files · +15 −11Browse files

Unified · Split

3 files
  1. gix-url/src/parse.rs +3 −1
  2. gix-url/tests/fixtures/fuzzed/very-long2.url +0 −0
  3. gix-url/tests/parse/mod.rs +13 −11
gix-url/src/parse.rs+3−1
@@ -85,7 +85,9 @@ pub(crate) fn find_scheme(input: &BStr) -> InputScheme { · 84 unmodified lines
85pub(crate) fn url(input: &BStr, protocol_end: usize) -> Result<crate::Url, Error> {85pub(crate) fn url(input: &BStr, protocol_end: usize) -> Result<crate::Url, Error> {
86 const MAX_LEN: usize = 1024;86 const MAX_LEN: usize = 1024;
87 let bytes_to_path = input[protocol_end + "://".len()..]87 let bytes_to_path = input[protocol_end + "://".len()..]
88 .find(b"/")88 .iter()
89 .skip_while(|b| **b == b'/')
90 .position(|b| *b == b'/')
89 .unwrap_or(input.len() - protocol_end);91 .unwrap_or(input.len() - protocol_end);
90 if bytes_to_path > MAX_LEN {92 if bytes_to_path > MAX_LEN {
91 return Err(Error::TooLong {93 return Err(Error::TooLong {
gix-url/tests/fixtures/fuzzed/very-long2.url+0−0

Binary file not shown.

gix-url/tests/parse/mod.rs+13−11
@@ -139,15 +139,17 @@ mod unknown { · 138 unmodified lines
139 139
140#[test]140#[test]
141fn fuzzed() {141fn fuzzed() {
142 let base = Path::new("tests").join("fixtures").join("fuzzed");142 for name in ["very-long", "very-long2"] {
143 let location = base.join(Path::new("very-long").with_extension("url"));143 let base = Path::new("tests").join("fixtures").join("fuzzed");
144 let url = std::fs::read(&location).unwrap();144 let location = base.join(Path::new(name).with_extension("url"));
145 let start = std::time::Instant::now();145 let url = std::fs::read(&location).unwrap();
146 gix_url::parse(url.as_bstr()).ok();146 let start = std::time::Instant::now();
147 assert!(147 gix_url::parse(url.as_bstr()).ok();
148 start.elapsed() < Duration::from_millis(100),148 assert!(
149 "URL at '{}' parsed too slowly, took {:.00}s",149 start.elapsed() < Duration::from_millis(100),
150 location.display(),150 "URL at '{}' parsed too slowly, took {:.00}s",
151 start.elapsed().as_secs_f32()151 location.display(),
152 )152 start.elapsed().as_secs_f32()
153 )
154 }
153}155}