search

modzero / mod0BurpUploadScanner

HTTP file upload scanner for Burp Proxy
Other
481 stars 138 forks source link

Improper Edge Side Include (ESI) Injection detection #11

Closed ldionmarcil closed 6 years ago

ldionmarcil commented 6 years ago

Hi, I reviewed your implementation of ESI detection (https://github.com/modzero/mod0BurpUploadScanner/blob/master/UploadScanner.py#L2146) and unfortunately, it's not effective against 99% of ESI-enabled surrogates. While SSRF is theoretically possible using ESI includes, the chance of includes being enabled for wildcard domains is near-nil, only Squid comes to mind right now. Most vendors will require manual domain whitelisting for ESI includes to work, and even the DNS won't get resolved (string compare is preferred over DNS resolution comparisons).

A more worthwhile approach is one comparable to the Burp Extension ActiveScanner++ (https://github.com/albinowax/ActiveScanPlusPlus/blob/master/activeScan%2B%2B.py#L381) where two ESI comment tags are inserted, one syntactically incorrect, and comparing to see if the correct one is stripped from the HTTP response. If the incorrect ESI tag is reflected and the proper one is not, then your backend is most likely ESI-enabled and injection is possible.

Example:

floyd-fuh commented 6 years ago

Ok, that sounds plausible, I'm not really familiar with ESI includes.

So your first example is potentially vulnerable, the second is for sure not, right?

What I can do is implement the "potentially vulnerable" check. However, if we would like to implement the "not vulnerable" check, the ReDownloader feature has to be configured (or rewrite how the code handles DownloadMatchers, which I don't want to do right now). Because the file content in the upload request is never reflected in the upload HTTP response directly, so we will need the ReDownloader to provide the "file content" when retrieved from the server right away... But the more robust solution and straight forward solution for now is to only do the "potentially vulnerable" check.

Long story short: Just implementing the first check and flag it as "potentially vulnerable" should be fine, right?

floyd-fuh commented 6 years ago

Nevermind the questions, I must have been a little confused, it's too hot here (it's both times the same test). Implementing right now (additionally to the Burp Collaborator test).

floyd-fuh commented 6 years ago

What do you think now? The ESI payload described is now also injected into every metadata type that is supported (e.g. JPEG files with XMP metadata). I haven't pushed it to the BApp store yet, let me know what you think.

ldionmarcil commented 6 years ago

I'm not setup to test your code right now, however the patch looks good. In due time I will test it against one of my test environments but feel free to push it to the app store.

floyd-fuh commented 6 years ago

It works for me, simply removed the ESI tag in one of the uploaded files and downloaded it and got an issue. Thanks for the feedback and happy hacking