amphp / http

HTTP primitives which can be shared by servers and clients.
https://amphp.org/http
MIT License
88 stars 10 forks source link

Cookie parsing strictness #5

Open kelunik opened 5 years ago

kelunik commented 5 years ago

Currently we're pretty strict about cookie parsing and return an empty cookie array if any cookie contains invalid data. Maybe we should change this, but it might result in security issues depending on the cookie.

bwoebi commented 5 years ago

Is there any real gain from this? Does it expose any real world issues?

kelunik commented 5 years ago

I hit this issue because some cookies from another application where present on localhost, probably not that important on deployed applications.

SUN5H1N3 commented 2 years ago

Is there any real gain from this? Does it expose any real world issues?

I ran into a problem that one of the sites was returning a cookie with a value containing spaces. As a result, this cookie was ignored and I received information on the site that was not what I wanted. If I add $value = urlencode($value) before preg_match(..., $value) to Amp\Http\Cookie::_construct() then the behavior becomes normal.

NoNamePaul94 commented 1 day ago

This actually became a real world issue for us today. We started using JENTIS for tracking an they actually put valid json in their cookie without encoding it: image (The redacted content are just numbers)

Since we don't really have control over this cookie but we still recieve the request with it in it, our application just breaks. Maybe add a json verification as an alternative to the regex or at least keep the valid cookies? (Also as mentioned above, I cannot fully grasp possible security implications)