Skip to content

strip control characters when writing the cookie header - #900

Open
dxbjavid wants to merge 1 commit into
apache:masterfrom
dxbjavid:cookie-header-control-chars
Open

dxbjavid wants to merge 1 commit into
apache:masterfrom
dxbjavid:cookie-header-control-chars

Conversation

@dxbjavid

Copy link
Copy Markdown
Contributor

RFC6265CookieSpec.formatCookies copies the cookie name and value into the Cookie request header without removing control characters. On the classic transport the header is serialised from the raw buffer of the BufferedHeader rather than its getValue(), so a cookie whose name or value was populated from attacker-influenced data and carries a CR or LF is written out verbatim and splits the Cookie line, which lets further request headers be injected. This replaces CR, LF and NUL with a space in the name and value before they are formatted, matching what BufferedHeader.getValue() already does when the value is read back and keeping cookie-name and cookie-value within what RFC 6265 permits.

@arturobernalg arturobernalg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One issue with the test: replacing CR and LF individually produces three spaces, including the original one, not one. Please adjust the expected value.

Also, replacing CR, LF, and NUL with spaces prevents this header injection vector, but does not guarantee compliance with the strict RFC 6265 cookie syntax.

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