From 2ddf08b85dd00f55707cf06a219e5d9dd8e975bb Mon Sep 17 00:00:00 2001 From: Chris Ross Date: Fri, 15 Apr 2022 15:03:08 -0700 Subject: [PATCH] Handle mismatched cookie chunk counts --- .../Infrastructure/ChunkingCookieManager.cs | 18 +++++++--- .../CookieChunkingTests.cs | 35 ++++++++++++++++++- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Owin/Infrastructure/ChunkingCookieManager.cs b/src/Microsoft.Owin/Infrastructure/ChunkingCookieManager.cs index 4c3f87cb7..fdf65bb9a 100644 --- a/src/Microsoft.Owin/Infrastructure/ChunkingCookieManager.cs +++ b/src/Microsoft.Owin/Infrastructure/ChunkingCookieManager.cs @@ -73,7 +73,7 @@ public string GetRequestCookie(IOwinContext context, string key) if (chunksCount > 0) { bool quoted = false; - string[] chunks = new string[chunksCount]; + var chunks = new List(10); // chunksCount may be wrong, don't trust it. for (int chunkId = 1; chunkId <= chunksCount; chunkId++) { string chunk = requestCookies[key + "C" + chunkId.ToString(CultureInfo.InvariantCulture)]; @@ -98,7 +98,8 @@ public string GetRequestCookie(IOwinContext context, string key) quoted = true; chunk = RemoveQuotes(chunk); } - chunks[chunkId - 1] = chunk; + + chunks.Add(chunk); } string merged = string.Join(string.Empty, chunks); if (quoted) @@ -236,13 +237,22 @@ public void DeleteCookie(IOwinContext context, string key, CookieOptions options List keys = new List(); keys.Add(escapedKey + "="); - string requestCookie = context.Request.Cookies[key]; - int 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++) { string subkey = escapedKey + "C" + 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 + "="); } } diff --git a/tests/Microsoft.Owin.Tests/CookieChunkingTests.cs b/tests/Microsoft.Owin.Tests/CookieChunkingTests.cs index 568d4d929..b2014fbe4 100644 --- a/tests/Microsoft.Owin.Tests/CookieChunkingTests.cs +++ b/tests/Microsoft.Owin.Tests/CookieChunkingTests.cs @@ -146,7 +146,7 @@ public void GetLargeChunkedCookieWithMissingChunk_ThrowingDisabled_NotReassemble public void DeleteChunkedCookieWithOptions_AllDeleted() { IOwinContext context = new OwinContext(); - context.Request.Headers.AppendValues("Cookie", "TestCookie=chunks:7"); + context.Request.Headers.AppendValues("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" }); var cookies = context.Response.Headers.GetValues("Set-Cookie"); @@ -163,5 +163,38 @@ public void DeleteChunkedCookieWithOptions_AllDeleted() "TestCookieC7=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT", }, cookies); } + + [Fact] + public void DeleteChunkedCookieWithMissingRequestCookies_OnlyPresentCookiesDeleted() + { + IOwinContext context = new OwinContext(); + 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.GetValues("Set-Cookie"); + Assert.Equal(3, cookies.Count); + Assert.Equal(new[] + { + "TestCookie=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure", + "TestCookieC1=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure", + "TestCookieC2=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure", + }, cookies); + } + + [Fact] + public void DeleteChunkedCookieWithMissingRequestCookies_StopsAtMissingChunk() + { + IOwinContext context = new OwinContext(); + // 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.GetValues("Set-Cookie"); + Assert.Equal(3, cookies.Count); + Assert.Equal(new[] + { + "TestCookie=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure", + "TestCookieC1=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure", + "TestCookieC2=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure", + }, cookies); + } } }