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

cookies: reject oversized cookies #1894

Closed
wants to merge 1 commit into from

Conversation

bagder
Copy link
Member

@bagder bagder commented Sep 17, 2017

... instead of truncating them.

There's no fixed limit for acceptable cookie names in RFC 6265, but the
entire cookie is said to be less than 4096 bytes (section 6.1).

Bug: https://curl.haxx.se/mail/lib-2017-09/0062.html
Reported-by: Kevin Smith

@bagder
Copy link
Member Author

bagder commented Sep 18, 2017

The travis failure is not because of the patch.

I will however make sure the code also refuses truncated content, and it we should check to see what the max cookie name size some popular browsers use so that we don't reject cookies that are generally accepted.

... instead of truncating them.

There's no fixed limit for acceptable cookie names in RFC 6265, but the
entire cookie is said to be less than 4096 bytes (section 6.1). This is
also what browsers seem to implement.

We now allow max 5000 bytes cookie header. Max 4095 bytes length per
cookie name and value. Name + value together may not exceed 4096 bytes.

Added test 1151 to verify

Bug: https://curl.haxx.se/mail/lib-2017-09/0062.html
Reported-by: Kevin Smith
@bagder bagder force-pushed the bagder/reject-oversized-cookies branch from a42400d to 992e99a Compare September 18, 2017 08:40
@jay
Copy link
Member

jay commented Sep 18, 2017

Chrome 61.0.3163.79 entire cookie (name=value) has a max length of 4096 and discards larger lengths.

edit; I see you were asking about the name specifically. You can have a name of 4094 in chrome.

@bagder
Copy link
Member Author

bagder commented Sep 18, 2017

Right, thanks. I noticed that Firefox has very similar logic as Chrome when it comes to the maximum cookie length. I used that as basis for selecting the curl limits used in the updated patch.

@bagder bagder closed this in 2bc230d Sep 18, 2017
@bagder bagder deleted the bagder/reject-oversized-cookies branch September 18, 2017 20:56
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants