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

[Request] - Add check for unencoded URI Path #3

Closed
zombietango opened this issue Dec 13, 2021 · 7 comments
Closed

[Request] - Add check for unencoded URI Path #3

zombietango opened this issue Dec 13, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@zombietango
Copy link

In my internal environment testing we've found some hosts/products that have been vulnerable to the Log4J vulnerability but where it would only fire if the URI path was NOT URL encoded. These hosts are not showing any vulnerable parameters when scanned using the plugin but will fire if I take the payload, un-encode it, and replay it in Repeater. When we check the logs generated, we found that it is logging the raw URI and this was causing the payload to not be interpreted by the vulnerable class. In these hosts, only the URI was vulnerable so the other non-encoded positions such as User-Agent were not being processed.

Thanks for putting this out, BTW!

@dnet dnet added the enhancement New feature or request label Dec 13, 2021
@v-p-b
Copy link
Contributor

v-p-b commented Dec 14, 2021

Correct me if I'm wrong, but I remember this being a limitation of Burp's Scanner (as it decides the encoding, and we can't really control it from an ext)?

As a workaround you can use Hackvertor and Intruder defined insertion points.

@zombietango
Copy link
Author

Oh, you know what you're probably right. I hadn't considered that. I have ways around it that we're already using, but I was hoping to rely on the plugin for automated detection by using some custom tools to feed requests into Burp and using a Live task to scan them all. That still is valuable, but we know that it won't be 100%.

@dnet
Copy link
Contributor

dnet commented Dec 14, 2021

The main issue is how the Burp scanner works: identifying insertion points (IScannerInsertionPointProvider) and scanning those insertion points (IScannerCheck) are two very distinct tasks done over two separate interfaces. Right now, Log4Shell scanner provides the latter by sending requests through the insertion points it receives. This way, the insertion point performs all the encoding (including nested ones like an attribute within a JSON that gets Base64 encoded and stuffed into a URL parameter).

So for your problem, the nice solution would be such an IScannerInsertionPointProvider that creates non-encoded insertion points within URL paths. It would be nice, because

  • there are more than just URL paths that might be interesting: think HTTP bodies, such as a logger that logs the wholex-www-form-urlencoded list of key-value pairs, JSON or XML, and
  • scans other than Log4Shell are also relevant, since such things might go into SQL databases as well.

That being said, based on #1, the good question is whether such an insertion point provider

  • needs to be written,
  • has already been written, or
  • is already part of Burp Suite Pro, we just haven't found it yet. :)

@v-p-b
Copy link
Contributor

v-p-b commented Dec 14, 2021

I would say this could be a nice, standalone extension that other scanners can rely on too, but should not be the scope of this one.

The ticket is useful to highlight such edge cases though, thanks for reporting!

@zombietango
Copy link
Author

Agreed. This does seem like a wider need and not specific to the Log4J vulnerability. It may be worth mentioning in the README that, at this time, there may be some edge cases that require manual/outside the plugin testing to confirm true negative states in terms of vulnerability to the issue.

@dnet dnet closed this as completed in 180d379 Dec 14, 2021
@pcderic
Copy link

pcderic commented Dec 16, 2021

Helpful description here. Not only did I learn something about the API, I also learned that the Intruder UI allows control over auditing insertion points from a note in the Burp Intruder docs :
"You can also use Intruder's payload positions UI to configure custom insertion points for scans by Burp Scanner. To do this, configure the request template and payload markers in the usual way within Intruder, and then select "Audit defined insertion points" from the Intruder menu. "

This might be worth mentioning in the README?

@dnet
Copy link
Contributor

dnet commented Dec 16, 2021

Helpful description here. Not only did I learn something about the API

Glad to hear 🎉

I also learned that the Intruder UI allows control over auditing insertion points

That's why @v-p-b wrote this above (emphasis mine):

As a workaround you can use Hackvertor and Intruder defined insertion points.

I'll think about how to mention this in the README

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

No branches or pull requests

4 participants