dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.35k stars 9.99k forks source link

QueryString key is not unescaped when value omitted #33394

Closed bart-degreed closed 3 years ago

bart-degreed commented 3 years ago

An incoming request with a query string that contains an escaped key without a value is not properly unescaped.

Example:

GET http://localhost/api/demo?fields%5BtodoItems%5D HTTP/1.1
string keys = string.Join(' ', new HttpContextAccessor().HttpContext.Request.Query.Keys);
// keys: %5BtodoItems%5D

In contrast, when the query string does contain a value, it gets unescaped properly.

GET http://localhost/api/demo?fields%5BtodoItems%5D=1 HTTP/1.1
string keys = string.Join(' ', new HttpContextAccessor().HttpContext.Request.Query.Keys);
// keys: [todoItems]

This bug applies to ASP.NET Core version: 3.1, 5.0 and the master branch.

The problem is caused by the next line: https://github.com/dotnet/aspnetcore/blob/52eff90fbcfca39b7eb58baad597df6a99a542b0/src/Http/WebUtilities/src/QueryHelpers.cs#L221 which does not unescape. To fix, replace this line with:

string name = queryString.Substring(scanIndex, delimiterIndex - scanIndex);
accumulator.Append(Uri.UnescapeDataString(name.Replace('+', ' ')), string.Empty);

When this gets fixed, it would be great to also backport it to .NET Core 3.1 and 5.0.

bart-degreed commented 3 years ago

I should note this problem does not repro when using WebApplicationFactory from an integration test, presumable because things are short-circuited in that case.