Scan rules don't manage correctly gzip compressed response content

[yhawke]

While doing a pentest I discovered that ZAP was unable to find an existing ExternalRedirect vulnerability.
after troubleshooting I discoverde that the original request is set using the Accept-Encoding: gzip header, so when the redirect occurs, the content given back by www.google.it (the site used for testing) is gzipped and the regexes don't work because the content isn't the real one.

Checking inside the ZAP panel implementation, I saw that the response visualization panel perform the ungzipping of the content while showing it... so everything seems working, but inside the core all plugins contine to working with an encoded content.

This mean that all the plugins could suffer about this implementation if the original request has the Accept-Encoding header set to gzip and the server supports it. Every time a msg.getResponseBody().toString() is performed the resulting string cannot be used correctly to search for regex...

[kingthorin]

Good find.

[psiinon]

Whats the best way to handle this?
Some options that spring to mind:

• Always removing 'Accept-Encoding: gzip' in reqs from the active scanner
• Automatically replacing the gzipped content with the uncompressed content on ascan responses
• Providing a ResponseBody.toUncompressedString() or similar method and checking all ascan rules use it

[kingthorin]

This isn't necessarily limited to active scan. Passive scanners may also be missing findings in compressed responses.

[psiinon]

Ugh :(
Maybe we should always return uncompressed content in ResponseBody.toString() and add a ResponseBody.getUncompressedContent() for code that needs it?

[thc202]

I'd go with toString() return the content uncompressed, always (and throw an exception if the encoding is not supported).
I don't think we need another method to obtain uncompressed content, it can be used getBytes().

BTW, this duplicates #408.

[thc202]

@yhawke was the option "Modify/Remove 'Accept-Encoding' request-header" [1] enabled when that happened?

[1] https://github.com/zaproxy/zap-core-help/wiki/HelpUiDialogsOptionsLocalproxy#modifyremove-accept-encoding-request-header

[kingthorin]

I like the suggestion that we always return uncompressed content in ResponseBody.toString()

[ElColmo]

If ResponseBody.toString() always returns the decompressed/inflated content, how would a plugin get the compressed/deflated content? Think about BREACH detection, for instance. Does "getBytes()" not return the de-compressed content? I thought it did.

As thc202 points out, the "Modify/Remove 'Accept-Encoding' request-header" may have been disabled, in which case, is this not expected functionality? (expect perhaps that the GUI should also respect the option, which it appears not to do)

Another option that might fix this issue:

• in the base active/passive scanner classes, have a method like "public boolean decompressCompressedResponse()", which can have an abstract implementation that returns true (which would apply for all scanners that choose not to over-ride the method). This option would then determine whether the response body is returned in compressed/decompressed format. Scanners that are actually interested in the actual raw (over the wire) response (compressed / decompressed / whatever) can override the method to return false. All other scanners would get the de-compressed response by default, which is probably what they actually want.

[kingthorin]

As thc202 points out, the "Modify/Remove 'Accept-Encoding' request-header" may have been disabled, in which case, is this not expected functionality? (expect perhaps that the GUI should also respect the option, which it appears not to do)

I can see arguments both ways. But my honest assumption was that an option for the local proxy to only applies to proxied traffic not necessarily the scanners (and ya I know what I get when I assume.....). (Think HTTPSender and source definition... kinda: if source == proxy then Modify/Remove.)
Sadly having written a few scanners I hadn't thought about the issue at all. I even made some modifications to the ExternalRedirect scanner that yhawke originally mentioned and didn't pickup on the issue.

Re: public boolean decompressCompressedResponse() this is an interesting idea but it may be possible at some future point that a single scanner might need to analyze both compressed and de-compressed responses. (I can't think of a circumstance right this second but it's not implausible....maybe the next BREACH attack relies on analysis of both or something....)

[yhawke]

Dear all, the option "Remove Accept Encoding" was enabled. Things don't work I suppose because it's not something related to the local proxy. I had this behavior while using the AJAX Spider because the Firefox browser manage things in this way.
And I think that removing this header is not a good option because we can have some sites that requires this kind of compression (I had this thing in the past on a mobile based application).
So I think that the good solution need to be something "transparent" to the plugins, in the same way happens for the output windows. So having the default getContent() methot that applies all the needed transformation, then a getBytes method that gives back the raw content.
In this way we can have two benefit: we don't need to change plugins, we solve the issue in an elegant way... If you want I can implement it in the same way we did for the Output Window.
Tell me your toughts on this

[kingthorin]

For reference: https://github.com/zaproxy/zaproxy/blob/29a69eaddfdf02e1df8fb922d1c58213fb66327f/src/org/zaproxy/zap/extension/httppanel/view/impl/models/http/response/ResponseStringHttpPanelViewModel.java

[pqyptixa]

Any updates on this? I just noticed that ZAP isn't actually removing the "Accept-Encoding:" header, it just removes gzip... but still sends "Accept-Encoding: br", so it receives "Content-Encoding: br".
BTW, I tried resending a GET request to a server that responds with gzipped(?) content, and ZAP (v2.5.0) just hung, right after clicking on "Send". Could not reproduce, though.

[kingthorin]

Brotli support ticket: #2198
Recent pull req related to these issues: #2570

[pqyptixa]

@kingthorin thanks!

[thc202]

@pqyptixa would you mind checking the log file to see if there's any error? (file zap.log located in ZAP's default directory or the directory manually specified [1]).

Was the GUI that hanged (i.e. no repaints were being done)?

[1] https://github.com/zaproxy/zaproxy/wiki/FAQconfig

[pqyptixa]

@thc202 No, there are no errors logged at the time ZAP hanged, in fact, there is nothing... I killed java a minute or so after ZAP hanged, and there is no indication of the hang.
And yes, the GUI hanged.

[thc202]

Merging into #408, which will make the scan rules handle this transparently (at least for text/string checks, scan rules that want to check the "raw" content can use the content as bytes, which should be already doing).

[thc202]

Correction, #408 not #403.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.