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

Remove cookie name decoding #368

Merged
merged 1 commit into from Aug 12, 2020
Merged

Remove cookie name decoding #368

merged 1 commit into from Aug 12, 2020

Conversation

Tratcher
Copy link
Member

@Tratcher Tratcher commented Jul 24, 2020

dotnet/aspnetcore#23578 dotnet/aspnetcore#24264

Decoding arbitrary input allows spoofing of special inputs like __Host- with encoded values like __%48ost-.

Fix: Only compare to known values. e.g. the key the developer passes in with the encoding we would have used.

This fix works seamlessly for the indexer but leaves the encoded value in enumerator.

Note Katana does not have quirks or AppContext switches. That should be OK because Katana is not bundled as part of a framework, it's an opt-in package upgrade.

This PR had to do a little more more work than the AspNetCore 2.1 version because Katana shared the same parser for cookies, forms, and query strings. The three are close, but there are a few differences people have complained about (aspnet/HttpAbstractions#547). AspNetCore changed to using a separate parser for each data type a long time ago.

TODO: versioning. This will probably be versioned as 4.1.1.

@Tratcher Tratcher marked this pull request as ready for review July 24, 2020 23:38
@Tratcher Tratcher requested review from blowdart and HaoK July 24, 2020 23:38
@Tratcher Tratcher self-assigned this Jul 24, 2020
@Tratcher Tratcher added this to the 4.1.1 milestone Jul 31, 2020
@Tratcher Tratcher merged commit 535ab4c into dev Aug 12, 2020
@Tratcher Tratcher deleted the tratcher/cookiename branch August 12, 2020 23:51
@bvmeer
Copy link

bvmeer commented Oct 7, 2020

We have a question related to this issue.
These prefixes are not used by default in ASP.NET Core or Microsoft.Owin libraries or templates.
It is not clear if we are vulnerable if we don't use cookies with the prefix mentioned. Thanks for adding more details.

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.

None yet

4 participants