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.

Sign up for GitHub

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

Jump to bottom

CR incorrectly permitted within header values #137

Closed
kenballus opened this issue Jun 25, 2024 · 1 comment · Fixed by #138
Closed

CR incorrectly permitted within header values #137

kenballus opened this issue Jun 25, 2024 · 1 comment · Fixed by #138

Comments

@kenballus
Copy link
Contributor

WEBrick allows CR (\r) within header values. RFC 9110 says not to do this:

Field values containing CR, LF, or NUL characters are invalid and dangerous, due to the varying ways that implementations might parse and interpret those characters; a recipient of CR, LF, or NUL within a field value MUST either reject the message or replace each of those characters with SP before further processing or forwarding of that message.

The suggested fix here would be to reject requests with headers containing bare CR.
jeremyevans added a commit to jeremyevans/webrick that referenced this issue Jun 25, 2024
@jeremyevans
Require CRLF line endings in request line and headers
9a12eed 
Disallow bare CR, LF, NUL in header and request lines. Tighten
parsing of request lines to only allow single spaces, as specified
in the RFCs.

Forcing this RFC-compliant behavior breaks a lot of tests, so
fix the tests to correctly use CRLF instead of LF for requests
(other than the specific checks for handling of bad requests).

Fixes ruby#137
@jeremyevans jeremyevans mentioned this issue Jun 25, 2024
Merged
@jeremyevans
Copy link
Contributor

@kenballus I submitted #138 to attempt to fix this, but it goes a little further than just disallowing CR in header values. Any chance you could review and let me know if I've made it stricter than what the RFCs allow?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Require CRLF line endings in request line and headers jeremyevans/webrick
2 participants
@jeremyevans @kenballus