Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Option to bypass validation during cookie serialization #191

Open
rosssteytler opened this issue Oct 10, 2024 · 11 comments
Open

Option to bypass validation during cookie serialization #191

rosssteytler opened this issue Oct 10, 2024 · 11 comments

Comments

@rosssteytler
Copy link

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?

@blakeembrey
Copy link
Member

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.

@rosssteytler
Copy link
Author

rosssteytler commented Oct 11, 2024

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.

@blakeembrey
Copy link
Member

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.

@rosssteytler
Copy link
Author

rosssteytler commented Oct 16, 2024

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.

@blakeembrey
Copy link
Member

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?

@blakeembrey
Copy link
Member

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.

@rosssteytler
Copy link
Author

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.

@blakeembrey
Copy link
Member

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 #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.

@GuyMevorachLevy
Copy link

Hey! Any news on that? All of our JSON cookies are broken now

@kirill-konshin
Copy link

kirill-konshin commented Nov 5, 2024

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.

@sgewirtz1
Copy link

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?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants
@blakeembrey @kirill-konshin @rosssteytler @sgewirtz1 @GuyMevorachLevy and others