luh2 / DetectDynamicJS

The DetectDynamicJS Burp Extension provides an additional passive scanner that tries to find differing content in JavaScript files and aid in finding user/session data.
GNU General Public License v3.0
65 stars 19 forks source link

Remove extension from passive scan #12

Closed soffensive closed 6 years ago

soffensive commented 6 years ago

Since the extension emits HTTP requests and is therefore anything but passive, I suggest to remove it from passive scans and invoke it only upon the user's express intent. Another reason to remove it from passive scans is that Burp's default behaviour is to scan any website passively (including websites that are not in scope).

For instance, it could be invoked with a dedicated context menu item.

luh2 commented 6 years ago

I am aware of that and it's stated in all presentations and also in the code. My intention was it running in the background, so I don't have to bother if there is any dynamic JavaScript or not. Invoking it manually is opposite to that idea. Maybe the context Menu wouldn't be to scan, but rather to enable dynamic JS detection and to disable it. But I am honestly fine with leaving it there as well. I had a short discussion about this with Dafydd, asking if he had a better idea, and he said that Burp's extension engine wasn't really built with extensions like this in mind, so that he thought the solution of warning the user is fine. I am a fan of simplicity, I guess that is why I just let it run in the background. Having to actively do something is a step to me forgetting to do so. Besides Google I have never run into problems with this extension. They log you out, if you request sources too often, that can only be accessed with proper authentication. How big of an issue you really think this is?

soffensive commented 6 years ago

My idea for an alternative was something like this: Target Tab -> Engagement Tools -> Find scripts > select all requests -> invoke the extension on them

Typically, this step would be carried out at the end of a test

soffensive commented 6 years ago

What about the following: The extension will check before becoming active whether or not the currently examined URL is in scope? cf. https://portswigger.net/Burp/extender/api/burp/IBurpExtenderCallbacks.html#isInScope(java.net.URL)

luh2 commented 6 years ago

As far as I understood, the passive scanner extensions will only be launched if the passive scanner is being run. If it is limited to a scope, the extension will automatically abide to it. Is that incorrect?

luh2 commented 6 years ago

I do mind is relying on Find scripts. As you might have seen, there are two further detection methods in the code that don't rely on the statedMimeType and inferredMimeType. It would be a shame to lose those. At least at time of development, it kept finding proper XSSI, that weren't detected by Burp.

I don't mind if this becomes an option in the drop down menu available in the proxy and target tab and instead be removed as passive scanner. Would you want to write that?

luh2 commented 6 years ago

I decided to let users give some input on this: https://twitter.com/fenceposterror/status/958280042785763328

luh2 commented 6 years ago

It was voted against changing it. I agree with the majority and will close this ticket.