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

[FEATURE REQUEST] Don't normalize URL #878

Closed
aroly opened this issue Apr 19, 2023 · 21 comments · Fixed by #884
Closed

[FEATURE REQUEST] Don't normalize URL #878

aroly opened this issue Apr 19, 2023 · 21 comments · Fixed by #884
Labels
enhancement New feature or request pinned

Comments

@aroly
Copy link

aroly commented Apr 19, 2023

My Problem

I'm doing content discovery on a host where there is a reverse proxy issue. The URL I would like to fuzz is https://x.x.x.x/FOO/../<FUZZ HERE>. In my case, the /FOO/ path is proxied to a backend host, then with the /../ I traverse directories on the final destination.

When I start feroxbuster with verbose output, I see that it normalizes the URL and uses https://x.x.x.x/<FUZZ HERE> which in my case is not proxied.

Describe the solution you'd like
An option to not normalize the URL. For example, curl as a --path-as-is option.

@aroly aroly added the enhancement New feature or request label Apr 19, 2023
@aroly aroly changed the title Feature Request - Don't normalize URL [Feature Request] Don't normalize URL Apr 19, 2023
@aroly aroly changed the title [Feature Request] Don't normalize URL [FEATURE REQUEST] Don't normalize URL Apr 19, 2023
@epi052 epi052 added good first issue Good for newcomers pinned labels Apr 19, 2023
@epi052
Copy link
Owner

epi052 commented Apr 19, 2023

hey there, thanks for this!

do you mean that the trailing slash is normalized, or that the path traversal string is normalized?

@aancw
Copy link
Contributor

aancw commented Apr 20, 2023

I think this issue is related to LFI based scanner, isn't? So it's better to add --path-as-is mode

@aroly
Copy link
Author

aroly commented Apr 20, 2023

hey there, thanks for this!

do you mean that the trailing slash is normalized, or that the path traversal string is normalized?

Hi,

The foo/../bar is normalized to /bar, which is the normal behaviour for a browser. But we are haxxors, right ? :) So an option to disable this normalization would be cool.

Antoine

@epi052 epi052 removed good first issue Good for newcomers pinned labels Apr 20, 2023
@epi052
Copy link
Owner

epi052 commented Apr 20, 2023

i spent some time checking this out this morning.

short answer: feroxbuster can't support this without significant work

ferox relies on the reqwest crate for its http client. In turn, reqwest depends on (and re-exports) the url crate.

The url crate has a Url type that, when .parse is called to create the Url, it both parses AND transforms the given url string. This parse function is what performs the normalization you're seeing. The url crate does not expose a way for the path to NOT be transformed on parse. There's been an open issue for a raw path open for nearly 3 years.

To compound the problem, the reqwest Client only works with Urls.

To summarize, to support this, we'd need to rip out all reqwest and url crate related code (quite a bit) AND replace it with something more flexible.

This isn't an effort I have the time to support right now, but will leave this open for now. I'd happily mentor/help someone through working on it.

@epi052 epi052 added the pinned label Apr 20, 2023
@aroly
Copy link
Author

aroly commented Apr 20, 2023

Thanks for the feedback, I totally understand your point of view.

Cheers,

A.

@lavafroth
Copy link
Contributor

lavafroth commented Apr 22, 2023

Hi there! There is a cheeky way to prevent truncating the .. in the URL by using the from_file_path function. Here's some crummy code I wrote to do so:

    let url_string = "http://127.0.0.1:8000/foo/../bar";
    let url_parsed = Url::parse(url_string).unwrap();
    let after_scheme = url_string.split_once("://").unwrap().1;
    let path = after_scheme.split_once('/').unwrap().1;
    let mut url = Url::from_file_path(format!("/{}", path)).unwrap();

    url.set_host(url_parsed.host_str()).unwrap();
    url.set_scheme(url_parsed.scheme()).unwrap();
    url.set_port(url_parsed.port()).unwrap();
    url.set_query(url_parsed.query());
    url.set_username(url_parsed.username()).unwrap();
    url.set_password(url_parsed.password()).unwrap();
    url.set_fragment(url_parsed.fragment());

    println!("{}", url); // outputs http://127.0.0.1:8000/foo/../bar

@epi052
Copy link
Owner

epi052 commented Apr 22, 2023

Lol! Ok, I'll give this a shot

@aancw
Copy link
Contributor

aancw commented Apr 22, 2023

Hi there! There is a cheeky way to prevent truncating the .. in the URL by using the from_file_path function. Here's some crummy code I wrote to do so:

    let url_string = "http://127.0.0.1:8000/foo/../bar";
    let url_parsed = Url::parse(url_string).unwrap();
    let after_scheme = url_string.split_once("://").unwrap().1;
    let path = after_scheme.split_once('/').unwrap().1;
    let mut url = Url::from_file_path(format!("/{}", path)).unwrap();

    url.set_host(url_parsed.host_str()).unwrap();
    url.set_scheme(url_parsed.scheme()).unwrap();
    url.set_port(url_parsed.port()).unwrap();
    url.set_query(url_parsed.query());
    url.set_username(url_parsed.username()).unwrap();
    url.set_password(url_parsed.password()).unwrap();
    url.set_fragment(url_parsed.fragment());

    println!("{}", url); // outputs http://127.0.0.1:8000/foo/../bar

When this input pass to reqwest, it still use .. or it normalized by reqwest?

@lavafroth
Copy link
Contributor

Yes, it will. I have tested that.

@aancw
Copy link
Contributor

aancw commented Apr 22, 2023

Yes, it will. I have tested that.

Nice job 👍👍👍

@lavafroth
Copy link
Contributor

I must warn you though, this is a pretty hacky way to accomplish the necessary URL path. Alternatively, we could use the hyper crate instead of reqwest which supports raw URLs.

@epi052
Copy link
Owner

epi052 commented Apr 23, 2023

writing a safer implementation of your hack is preferable to me at the moment. replacing the reqwest client with hyper isn't something I have time to do at the moment. Writing a single function that the Url::parse call sites can uniformly use isn't too bad, lol.

However, I think, long term, i'd opt for a tower-http wrapped hyper client. That would allow for the flexibility of a hyper client as well as facilitate extensibility of the hyper client via tower.

@epi052
Copy link
Owner

epi052 commented Apr 23, 2023

ok, here's somethign pretty close to what i'd include in ferox. if you get some time, i'd appreciate it if yall could give it a look and see if i missed anything

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=fac606cd7126f71f03fb1b715edc6c76

the ParsedUrl stuff is really just to facilitate a couple of different uses at different callsites aroudn teh code base

@epi052 epi052 mentioned this issue Apr 24, 2023
17 tasks
@epi052
Copy link
Owner

epi052 commented Apr 27, 2023

@all-contributors add @aroly for ideas, @lavafroth for code and ideas

@allcontributors
Copy link
Contributor

@epi052

I've put up a pull request to add @aroly! 🎉

@epi052
Copy link
Owner

epi052 commented Apr 27, 2023

@all-contributors add @aroly for ideas

@allcontributors
Copy link
Contributor

@epi052

@aroly already contributed before to ideas

@epi052
Copy link
Owner

epi052 commented Apr 27, 2023

@all-contributors add @lavafroth for code and ideas

@allcontributors
Copy link
Contributor

@epi052

This project's configuration file has malformed JSON: .all-contributorsrc. Error:: Unexpected token ] in JSON at position 15321

@epi052
Copy link
Owner

epi052 commented Apr 27, 2023

@all-contributors add @lavafroth for code and ideas

@allcontributors
Copy link
Contributor

@epi052

I've put up a pull request to add @lavafroth! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pinned
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants