Fix numerous message parsing issues (v2) #3113
Conversation
pajod
force-pushed
the
patch-security
branch
from
b78b7e0
to
8874620
Compare
|
Further changes added:
|
and unbreak test depending on backslash escape
* update HEADER_RE and HEADER_VALUE_RE to match the RFCs * update chunk length parsing to disallow 0x prefix and digit-separating underscores.
- Unify HEADER_RE and METH_RE - Replace CRLF with SP during obs-fold processing (See RFC 9112 Section 5.2, last paragraph) - Stop stripping header names. - Remove HTAB in OWS in header values that use obs-fold (See RFC 9112 Section 5.2, last paragraph) - Use fullmatch instead of search, which has problems with empty strings. (See GHSA-68xg-gqqm-vgj8) - Split proxy protocol line on space only. (See proxy protocol Section 2.1, bullet 3) - Use fullmatch for method and version (Thank you to Paul Dorn for noticing this.) - Replace calls to str.strip() with str.strip(' \t') - Split request line on SP only. Co-authored-by: Paul Dorn <pajod@users.noreply.github.com>
* refusing lowercase and ASCII 0x23 (#) had been partially enforced before * do not casefold by default, HTTP methods are case sensitive
Ambiguous mappings open a bottomless pit of "what is user input and what is proxy input" confusion. Default to what everyone else has been doing for years now, silently drop. see also https://nginx.org/r/underscores_in_headers
Somehow exception logging was conditional on successful request uri parsing. Add it back for the other branch.
If we promise wsgi.input_terminated, we better get it right - or not at all. * chunked encoding on HTTP <= 1.1 * chunked not last transfer coding * multiple chinked codings * any unknown codings (yes, this too! because we do not detect unusual syntax that is still chunked) * empty coding (plausibly harmless, but not see in real life anyway - refused, for the moment)
In common configuration unlikely a big security problem in itself you are just fooling the remote about https. However, it is offers an oracle for otherwise invisible proxy request headers, so it might help exploiting other vulnerabilities.
Do the validation on the original, not the result from unicode case folding. Background: latin-1 0xDF is traditionally uppercased 0x53+0x53 which puts it back in ASCII
Note: This is unrelated to a reverse proxy potentially talking HTTP/3 to clients. This is about the HTTP protocol version spoken to Gunicorn, which is HTTP/1.0 or HTTP/1.1. Little legitimate need for processing HTTP 1 requests with ambiguous version numbers. Broadly refuse. Co-authored-by: Ben Kallus <benjamin.p.kallus.gr@dartmouth.edu>
further information to be published in security advisories, published out of tree on Github
pajod
force-pushed
the
patch-security
branch
from
8874620
to
b6c7414
Compare
chunk extensions are silently ignored before and after this change; its just the whitespace handling for the case without extensions that matters applying same strip(WS)->rstrip(BWS) replacement as already done in related cases half-way fix: could probably reject all BWS cases, rejecting only misplaced ones
|
LGTM. I am deploying it in semi prod to check. |
|
If all things look good, could we please have a new release? Thank you! |
@benoitc, icymi. Thanks! |
|
@benoitc firstly thanks for all your work in maintaining and developing gunicorn . It is a great piece of software. This has now hit the vulnerability databases and so it is starting to block downstream pipelines. If there is some way we could expedite this fix to a release it would be much appreciated. |
You can install from git e.x. https://github.com/nijave/webhook-dyn-dnsimple/pull/11/files requirements.txt |
|
For those tracking this issue, version 22.0.0 has now been published |
|
@nijave thanks for this tip this would have worked until this release. @WPettersson noted thanks. Thanks to all maintainers and contributors for dealing with this so quickly. |
Contains non-trivially-merged changes:
Contains rebased & Updated version of these PRs:
Therefore:
Transfer-Encodingheader values #3087