Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: problematic parsing leniency in parsing chunk extensions #3327

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JeppW
Copy link

@JeppW JeppW commented Nov 24, 2024

Stop allowing newline characters in chunk extensions. This can cause request smuggling issues with some reverse proxies.

Reference: https://grenfeldt.dev/2021/10/08/gunicorn-20.1.0-public-disclosure-of-request-smuggling/

@pajod
Copy link
Contributor

pajod commented Nov 24, 2024

Any thoughts on singling out just the newline, among multiple characters that could conceivably cause trouble behind a robustdangerous proxy?

@JeppW
Copy link
Author

JeppW commented Nov 24, 2024

AFAIK only the newline has ever been shown to be exploitable in practice. Perhaps we should disallow \r as well, it seems plausible that a proxy could misinterpret \rX as a line terminator. Other than that, I think further validation would just be unnecessary overhead.

@JeppW
Copy link
Author

JeppW commented Nov 26, 2024

I added the \r as we agreed. Can you run the workflows?

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.

2 participants