Merged PR 22110: [3.1] Fix partial chunked cookies 70242

# Fix partial chunked cookies

MSRC # 70242: Fix exceptions and allocations when the cookie chunk count is not accurate

Backported from 6.0: https://dev.azure.com/dnceng/internal/_git/dotnet-aspnetcore/pullrequest/21247

## Description

Browsers have limits on how long cookies can be, as low as 4kb. It's common for TempData and CookieAuth to get above that limit, so cookies are split into chunks with the following format:

MyCookie=chunks-3
MyCookieC1=(Base64EncodedData)
MyCookieC2=(Base64EncodedData)
MyCookieC3=(Base64EncodedData)

Fixes MSRC # 70242

## Customer Impact

A malicious client could send `MyCookie=chunks-2147483647` without the actual cookie chunks and cause large allocations, exceptions, and excess CPU utilization on the server when it tried to read or delete that many chunks.

This flaw comes from the original implementation in Microsoft.Owin, but is much worse in AspNetCore when adopted by TempData due to it automatically calling Delete if reading the cookie fails.

## Regression?

- [ ] Yes
- [x] No

## Risk

- [ ] High
- [ ] Medium
- [x] Low

Easy to reproduce and test.

## Verification

- [x] Manual (required)
- [x] Automated

## Packaging changes reviewed?

- [ ] Yes
- [ ] No
- [x] N/A

Author: Tratcher

src/Security/CookiePolicy/test/CookieChunkingTests.cs

@@ -3,6 +3,7 @@
 
 using System;
 using Microsoft.AspNetCore.Http;
+using Microsoft.Net.Http.Headers;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Internal
@@ -30,7 +31,7 @@ public void AppendLargeCookie_WithOptions_Appended()
             {
                 Domain = "foo.com",
                 HttpOnly = true,
-                SameSite = SameSiteMode.Strict,
+                SameSite = Http.SameSiteMode.Strict,
                 Path = "/bar",
                 Secure = true,
                 Expires = now.AddMinutes(5),
@@ -128,7 +129,7 @@ public void GetLargeChunkedCookieWithMissingChunk_ThrowingDisabled_NotReassemble
         public void DeleteChunkedCookieWithOptions_AllDeleted()
         {
             HttpContext context = new DefaultHttpContext();
-            context.Request.Headers.Append("Cookie", "TestCookie=chunks-7");
+            context.Request.Headers.Append("Cookie", "TestCookie=chunks-7;TestCookieC1=1;TestCookieC2=2;TestCookieC3=3;TestCookieC4=4;TestCookieC5=5;TestCookieC6=6;TestCookieC7=7");
 
             new ChunkingCookieManager().DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com", Secure = true });
             var cookies = context.Response.Headers["Set-Cookie"];
@@ -145,5 +146,92 @@ public void DeleteChunkedCookieWithOptions_AllDeleted()
                 "TestCookieC7=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
             }, cookies);
         }
+
+        [Fact]
+        public void DeleteChunkedCookieWithMissingRequestCookies_OnlyPresentCookiesDeleted()
+        {
+            HttpContext context = new DefaultHttpContext();
+            context.Request.Headers.Append("Cookie", "TestCookie=chunks-7;TestCookieC1=1;TestCookieC2=2");
+
+            new ChunkingCookieManager().DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com", Secure = true });
+            var cookies = context.Response.Headers["Set-Cookie"];
+            Assert.Equal(3, cookies.Count);
+            Assert.Equal(new[]
+            {
+                "TestCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC1=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC2=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+            }, cookies);
+        }
+
+        [Fact]
+        public void DeleteChunkedCookieWithMissingRequestCookies_StopsAtMissingChunk()
+        {
+            HttpContext context = new DefaultHttpContext();
+            // C3 is missing so we don't try to delete C4 either.
+            context.Request.Headers.Append("Cookie", "TestCookie=chunks-7;TestCookieC1=1;TestCookieC2=2;TestCookieC4=4");
+
+            new ChunkingCookieManager().DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com", Secure = true });
+            var cookies = context.Response.Headers["Set-Cookie"];
+            Assert.Equal(3, cookies.Count);
+            Assert.Equal(new[]
+            {
+                "TestCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC1=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC2=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+            }, cookies);
+        }
+
+        [Fact]
+        public void DeleteChunkedCookieWithOptionsAndResponseCookies_AllDeleted()
+        {
+            var chunkingCookieManager = new ChunkingCookieManager();
+            HttpContext httpContext = new DefaultHttpContext();
+
+            httpContext.Request.Headers["Cookie"] = new[]
+            {
+                "TestCookie=chunks-7",
+                "TestCookieC1=abcdefghi",
+                "TestCookieC2=jklmnopqr",
+                "TestCookieC3=stuvwxyz0",
+                "TestCookieC4=123456789",
+                "TestCookieC5=ABCDEFGHI",
+                "TestCookieC6=JKLMNOPQR",
+                "TestCookieC7=STUVWXYZ"
+            };
+
+            var cookieOptions = new CookieOptions()
+            {
+                Domain = "foo.com",
+                Path = "/",
+                Secure = true
+            };
+
+            httpContext.Response.Headers[HeaderNames.SetCookie] = new[]
+            {
+                "TestCookie=chunks-7; domain=foo.com; path=/; secure",
+                "TestCookieC1=STUVWXYZ; domain=foo.com; path=/; secure",
+                "TestCookieC2=123456789; domain=foo.com; path=/; secure",
+                "TestCookieC3=stuvwxyz0; domain=foo.com; path=/; secure",
+                "TestCookieC4=123456789; domain=foo.com; path=/; secure",
+                "TestCookieC5=ABCDEFGHI; domain=foo.com; path=/; secure",
+                "TestCookieC6=JKLMNOPQR; domain=foo.com; path=/; secure",
+                "TestCookieC7=STUVWXYZ; domain=foo.com; path=/; secure"
+            };
+
+            chunkingCookieManager.DeleteCookie(httpContext, "TestCookie", cookieOptions);
+            Assert.Equal(8, httpContext.Response.Headers[HeaderNames.SetCookie].Count);
+            Assert.Equal(new[]
+            {
+                "TestCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC1=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC2=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC3=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC4=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC5=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC6=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+                "TestCookieC7=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure"
+            }, httpContext.Response.Headers[HeaderNames.SetCookie]);
+        }
     }
 }

src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs

@@ -99,7 +99,7 @@ public string GetRequestCookie(HttpContext context, string key)
             var chunksCount = ParseChunksCount(value);
             if (chunksCount > 0)
             {
-                var chunks = new string[chunksCount];
+                var chunks = new List<string>(10); // chunksCount may be wrong, don't trust it.
                 for (var chunkId = 1; chunkId <= chunksCount; chunkId++)
                 {
                     var chunk = requestCookies[key + ChunkKeySuffix + chunkId.ToString(CultureInfo.InvariantCulture)];
@@ -124,7 +124,7 @@ public string GetRequestCookie(HttpContext context, string key)
                         return value;
                     }
 
-                    chunks[chunkId - 1] = chunk;
+                    chunks.Add(chunk);
                 }
 
                 return string.Join(string.Empty, chunks);
@@ -241,13 +241,22 @@ public void DeleteCookie(HttpContext context, string key, CookieOptions options)
             var keys = new List<string>();
             keys.Add(key + "=");
 
-            var requestCookie = context.Request.Cookies[key];
-            var chunks = ParseChunksCount(requestCookie);
+            var requestCookies = context.Request.Cookies;
+            var requestCookie = requestCookies[key];
+            long chunks = ParseChunksCount(requestCookie);
             if (chunks > 0)
             {
                 for (int i = 1; i <= chunks + 1; i++)
                 {
                     var subkey = key + ChunkKeySuffix + i.ToString(CultureInfo.InvariantCulture);
+
+                    // Only delete cookies we received. We received the chunk count cookie so we should have received the others too.
+                    if (string.IsNullOrEmpty(requestCookies[subkey]))
+                    {
+                        chunks = i - 1;
+                        break;
+                    }
+
                     keys.Add(subkey + "=");
                 }
             }