Skip to content

sanitize transfer-encoding and reject malformed "chunked"#617

Merged
rofl0r merged 5 commits into
tinyproxy:masterfrom
rofl0r:chunked_again
Jun 26, 2026
Merged

sanitize transfer-encoding and reject malformed "chunked"#617
rofl0r merged 5 commits into
tinyproxy:masterfrom
rofl0r:chunked_again

Conversation

@rofl0r

@rofl0r rofl0r commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

addressing #616 (yet another instance of request smuggling)

addressing tinyproxy#616 (yet another instance of request smuggling)
@rofl0r

rofl0r commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

@afldl is your frontier model of the opinion this fix is sufficient?

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens tinyproxy’s HTTP/1.1 request handling against request smuggling (issue #616) by sanitizing and validating Transfer-Encoding so that malformed or ambiguous chunked values can’t bypass the existing CL/TE protections.

Changes:

  • Replaces the old exact-match Transfer-Encoding: chunked check with an in-place sanitizer/parser that attempts to validate chunked placement.
  • Rejects malformed Transfer-Encoding with an HTTP 400 response.
  • Continues to strip Content-Length when a valid chunked framing is detected (to prevent CL/TE desync).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/reqs.c
Comment thread src/reqs.c
Comment thread src/reqs.c
Comment thread src/reqs.c Outdated
Comment thread src/reqs.c Outdated
Comment thread src/reqs.c Outdated
@afldl

afldl commented Jun 24, 2026

Copy link
Copy Markdown

you are right

@afldl

afldl commented Jun 24, 2026

Copy link
Copy Markdown

I'd like to ask whether the issue described below qualifies for a CVE ID, and what (if anything) I should prepare before submitting a formal request.

@rofl0r

rofl0r commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

you are right

does that mean that the PR was classified as sufficient and RFC conform?

would you mind running the test (with asan) you used in #616 again on this PR ?

I'd like to ask whether the issue described below qualifies for a CVE ID,

if the previous iteration had one than i guess it's only logical this one gets one too

what (if anything) I should prepare before submitting a formal request.

i'm not familiar with CVE submission processes, but i was of the impression it's pretty straight-forward.

@afldl

afldl commented Jun 25, 2026

Copy link
Copy Markdown

Verified the fix on PR HEAD (d95fc6d) with a real ASAN/UBSan build over real TCP — all four bypass vectors from #616 are closed, Content-Length is correctly stripped on the canonical chunked case, malformed/multi-coding values are rejected with 400, and the run is clean. Looks good to me — resolves #616. Thanks @rofl0r!

@rofl0r rofl0r merged commit 7bc33a7 into tinyproxy:master Jun 26, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants