[squid-dev] [PATCH] Reject or sanitize more problematic Content-Length values

Amos Jeffries squid3 at treenet.co.nz
Fri Sep 2 15:05:44 UTC 2016


On 2/09/2016 11:21 a.m., Alex Rousskov wrote:
> Hello,
> 
>     Squid is violating HTTP MUSTs by forwarding messages with
> problematic Content-Length values. Some of those bugs were fixed in
> trunk r14215. This change handles multiple Content-Length values inside
> one header field, negative values, and trailing garbage. Handling the
> former required a change in the overall Content-Length interpretation
> approach (which is why it was previously left as a TODO).
> 
> Squid now passes almost all Co-Advisor tests devoted to this area. We
> are not 100% done though: We still need to handle malformed values with
> leading signs (e.g., "-0" or "+1"). However, I hope that the remaining
> problems are relatively minor. I do not plan on addressing them in the
> foreseeable future.
> 
> Also improved httpHeaderParseOffset(): Added detection of overflowing
> and underflowing integer values; polished malformed value detection code
> (Linux strtoll(3) manual page has a good example). The function no
> longer considers empty strings valid and reports trailing characters.
> The function still accepts leading whitespace and signs. It is still the
> wrong approach to HTTP numbers parsing, but further improvements are out
> of scope because they are complicated and would require significant
> caller rewrites.
> 

+1. Looks okay to me. With one caveat:

Since there are several of these interpreters for special-cased headers
I think we need to define a sub-namespace for header
interpreters/parsers, etc. Does keeping the existing "Hdr" term seem
like a good idea to you?

* If so please move ContentLengthInterpreter to a Http::Hdr:: namespace
and ContentLengthInterpreter.{h,cc} files in a matching directory.

* Otherwise, please at least place the class within the Http:: namespace
and directory etc.


(PS. I plan to bundle next 4.x within the next 2-3 days. I leave it to
you to decide whether this gets applied now or after release).

Amos



More information about the squid-dev mailing list