Skip to content

begin <= end (4 <= 2) when slicing a://a in Url::username #397

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

Closed
mateon1 opened this issue Oct 1, 2017 · 2 comments
Closed

begin <= end (4 <= 2) when slicing a://a in Url::username #397

mateon1 opened this issue Oct 1, 2017 · 2 comments
Labels

Comments

@mateon1
Copy link

mateon1 commented Oct 1, 2017

Found by fuzzing.

let url = Url::parse("a:/a/..//a").unwrap()
url.username(); // panic
thread 'main' panicked at 'begin <= end (4 <= 2) when slicing `a://a`', /checkout/src/libcore/str/mod.rs:2179:4
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:381
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:397
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:577
   5: std::panicking::begin_panic
             at /checkout/src/libstd/panicking.rs:538
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:522
   7: rust_begin_unwind
             at /checkout/src/libstd/panicking.rs:498
   8: core::panicking::panic_fmt
             at /checkout/src/libcore/panicking.rs:71
   9: core::str::slice_error_fail
             at /checkout/src/libcore/str/mod.rs:2179
  10: core::str::traits::<impl core::slice::SliceIndex<str> for core::ops::range::Range<usize>>::index::{{closure}}
             at /checkout/src/libcore/str/mod.rs:1850
  11: <core::option::Option<T>>::unwrap_or_else
             at /checkout/src/libcore/option.rs:370
  12: core::str::traits::<impl core::slice::SliceIndex<str> for core::ops::range::Range<usize>>::index
             at /checkout/src/libcore/str/mod.rs:1850
  13: core::str::traits::<impl core::ops::index::Index<core::ops::range::Range<usize>> for str>::index
             at /checkout/src/libcore/str/mod.rs:1624
  14: <core::ops::range::Range<u32> as url::RangeArg>::slice_of
             at src/lib.rs:2160
  15: url::Url::slice
             at src/lib.rs:2066
  16: url::Url::username
             at src/lib.rs:705
  17: testurl::test
             at ./testurl.rs:14
  18: testurl::main
             at ./testurl.rs:38
  19: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:99
  20: std::rt::lang_start
             at /checkout/src/libstd/panicking.rs:459
             at /checkout/src/libstd/panic.rs:361
             at /checkout/src/libstd/rt.rs:59
  21: main
  22: __libc_start_main
  23: _start
@windwarrior
Copy link

windwarrior commented Feb 13, 2018

I have tried (and so far failed) to find and fix this issue, here is what I have found:

The variable username_end is set here to 2, and the parser (I think) rightfully assumes that this URL has no authority. The remainder of the URL is parsed as a path in which some form of truncation happens to a://a.

The problem now is that the (truncated) URL is a://a, which is considered an URL with authority in has_authority since the truncated URL does start (after the scheme) with ://. The username function then attempts to extract this username, but username_end is set to 2 because the parser didn't think there was an autorithy section in this URL.

I think its a mismatch between parser and lib code, the former thinking there is no authority section and the latter thinking there is, but have too limited of an understanding of the codebase to decide which of the two is in the wrong. I do hope however that my little bughunt helps you solve the issue!

Furthermore, I have found that the testcase presented in this issue is not minimal, the url a:/..// also crashes for similar reasons.

zealousidealroll pushed a commit to zealousidealroll/rust-url that referenced this issue Sep 6, 2018
This avoids a contradiction where the URL starts out with no host,
but gets normalized into a URL where the first part of a path is
interpreted as the host.

Fixes servo#397
zealousidealroll added a commit to zealousidealroll/rust-url that referenced this issue Sep 6, 2018
This avoids a contradiction where the URL starts out with no host,
but gets normalized into a URL where the first part of a path is
interpreted as the host.

Fixes servo#397
zealousidealroll added a commit to zealousidealroll/rust-url that referenced this issue Sep 6, 2018
This avoids a contradiction where the URL starts out with no host,
but gets normalized into a URL where the first part of a path is
interpreted as the host.

Fixes servo#397
zealousidealroll pushed a commit to zealousidealroll/rust-url that referenced this issue Sep 6, 2018
This avoids a contradiction where the URL starts out with no host,
but gets normalized into a URL where the first part of a path is
interpreted as the host.

Fixes servo#397
zealousidealroll added a commit to zealousidealroll/rust-url that referenced this issue Sep 6, 2018
This avoids a contradiction where the URL starts out with no host,
but gets normalized into a URL where the first part of a path is
interpreted as the host.

Fixes servo#397
@SimonSapin SimonSapin added the bug label Oct 17, 2018
zealousidealroll pushed a commit to zealousidealroll/rust-url that referenced this issue Dec 11, 2018
This is a willfull violation of the URL specification for serialization.
It retains spec-compliance for parsing, it aligns with the behavior
of Microsoft Edge, and it "fixes" an acknowledged specification bug.

See whatwg/url#415

Fixes servo#397
zealousidealroll pushed a commit to zealousidealroll/rust-url that referenced this issue Dec 11, 2018
This is a willfull violation of the URL specification for serialization.
It retains spec-compliance for parsing, it aligns with the behavior
of Microsoft Edge, and it "fixes" an acknowledged specification bug.

See whatwg/url#415

Fixes servo#397
zealousidealroll added a commit to zealousidealroll/rust-url that referenced this issue Dec 11, 2018
This is a willfull violation of the URL specification for serialization.
It retains spec-compliance for parsing, it aligns with the behavior
of Microsoft Edge, and it "fixes" an acknowledged specification bug.

See whatwg/url#415

Fixes servo#397
@qsantos
Copy link
Contributor

qsantos commented Mar 19, 2023

It looks like this was resolved by 9aac190 or #655 by @djc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants