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

Document security considerations: "CRLF in headers" #8964

Closed
wants to merge 8 commits into from

Conversation

Haxatron
Copy link
Contributor

@Haxatron Haxatron commented Jun 6, 2022

@jay
Copy link
Member

jay commented Jun 6, 2022

I don't think this warning is really needed but if it is does it really need two paragraphs?

@Haxatron
Copy link
Contributor Author

Haxatron commented Jun 7, 2022

Hi, the rationale (from the hacker1 report) was that untrusted user input that contain newlines could be used to perform all sorts of bad stuff. This has appeared before, such as in https://medium.com/@tomnomnom/crlf-injection-into-phps-curl-options-e2e0d7cfe545.

This update to the documentation aims to address that by informing users that libcurl will not sanitize headers, and that user have to sanitize headers by themselves. However, after thinking about it for a while, I think only libcurl-security.3 should have this warning, to reduce clutter in the options documentation. wdyt?

@jay jay closed this in 23408f1 Jun 8, 2022
@jay
Copy link
Member

jay commented Jun 8, 2022

This update to the documentation aims to address that by informing users that libcurl will not sanitize headers, and that user have to sanitize headers by themselves. However, after thinking about it for a while, I think only libcurl-security.3 should have this warning, to reduce clutter in the options documentation. wdyt?

I agree. I just landed that and I changed the warning sentence slightly so that it is broader than just new headers injection: "someone malicious may be able to modify the request in a way you didn't intend such as injecting new headers." Someone malicious could do \r\n\r\n and inject into the body, and there are probably other things I'm not thinking of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants