Closed rosssteytler closed 2 days ago
I think it's possible, but it does re-introduce the vulnerability from 0.6 and below. Do you have an example of something that's failing so I can get a sense of what needs to skip validation? I'd prefer to avoid re-introducing a potential issue into the library.
In this particular case it is a cookie value that contains a comma. This was working on version 0.6 but restricted in the current version.
In this particular case it is a cookie value that contains a comma.
Are you also trying to avoid encoding the comma? What's the decode/encode functions you use? Because this library, by default, would be escaping the comma to %2C
.
The cookie is initially created by the origin server (not by me). In my proxy workflow, I use the cookie library to parse it, modify configurations like expires
, and then reserialize it. Since I'm not the origin of the cookie, I can't alter its name or value, so I've disabled decoding and encoding. I just need the cookie to pass through my proxy as is, regardless of its validity - hence the need for an option to bypass validation in this particular scenario.
I use the cookie library to parse it
Out of curiosity, how are you parsing it with this library? It was only designed to parse the cookie
header so I'm guessing you had to build out some other things?
Since I'm not the origin of the cookie, I can't alter its name or value, so I've disabled decoding and encoding.
Do you know if it's only the comma in the cookie value? I'm inclined to allow it to be a bit looser to fit what browsers support as long as it doesn't create a security issue. But bypassing everything is a little iffier due to the ability to escape field boundaries.
Out of curiosity, how are you parsing it with this library? It was only designed to parse the
cookie
header so I'm guessing you had to build out some other things?
My proxy service receives a request intended for a target server. The proxy service then makes a fetch request to the target server and parses the raw set-cookie response headers from the target server using this library. Once I've modified certain options such as expires
I reserialize the cookie using this library and return it in the response to the original request.
Do you know if it's only the comma in the cookie value?
In the short amount of testing I did before rolling back to the previous version of the library I only noticed a comma breaking.
The proxy service then makes a fetch request to the target server and parses the raw set-cookie response headers from the target server using this library.
But won't you have issues like https://github.com/jshttp/cookie/issues/200? I'm basically trying to figure out if you need a new method for what you're doing, because I worry trying to parse with this library will hit random issues (case sensitivity, wrong types, etc).
I only noticed a comma breaking.
Awesome, thanks. I'll re-enable the comma, I was looking around and IIRC it might be .NET or another language that was allowing these characters. I'm OK being more permissive as long as it's not introducing a security issue.
Hey! Any news on that? All of our JSON cookies are broken now
I have an example: library Shadcn-ui sets a cookie with name "sidebar:state":
https://github.com/shadcn-ui/ui/blob/d64374d009dabfbfb1bb9ad6ffa8d1973879e8f6/apps/www/registry/default/ui/sidebar.tsx#L22 https://github.com/shadcn-ui/ui/blob/d64374d009dabfbfb1bb9ad6ffa8d1973879e8f6/apps/www/registry/default/ui/sidebar.tsx#L87
I also have a scenario where I parse and serialize cookies in a proxy. This works OK without a proxy, browsers and Next.js do accept such names.
We're from another team (approuter library) which is using the cookies library where most of our customers are all failing due to illegal characters (including commas) we are using the cookies library, is there no way to allow bypass of this validation in favor of the prior check?
We are writing a custom proxy server for a customer, and currently we can't use this library because of the validation that fails. The upstream server sets a cookie called csrf[frontend.checkout.switch-language]
, which fails the regex match when we try to forward this cookie to the client.
I have a PR https://github.com/jshttp/cookie/pull/210 which will resolve this issue, however it'd be useful to get some more information on some things:
All of our JSON cookies are broken now
Are you skipping encode
and decode
? Can you share how you are setting JSON cookies? It's not particularly recommended to just JSON only since you'll have other invalid characters that can create exploitable cookies if you allow ;
.
no way to allow bypass of this validation in favor of the prior check?
By passing validation isn't allowed because it creates a vulnerability if you are serializing user data, and they can do something like x;Expires=0;
to unset a cookie from the cookie value. I'm trying instead to loosen the validation without introducing a security issue for users.
@blakeembrey thanks a lot, great news!
I'm trying instead to loosen the validation without introducing a security issue for users.
Way to go.
The recent updates to introduce stricter validation on cookie names and values has broken some workflows in proxy scenarios where I'm parsing, modifying and then reserializing cookies.
In this case I am not the origin of the cookies and stricter validation is not required. It would be very helpful if there was a new option to bypass the validation on name and value during serialization.
Is this something you would consider adding to the library?