Skip to content

CookiePolicyMiddleware modifies CookieOptions, leading to bugs if it's reused #53875

@kimsey0

Description

@kimsey0

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

The CookiePolicyMiddleware that's set up with UseCookiePolicy directly modifies the CookieOptions that's passed in to IResponseCookies.Append. This can lead to bugs where the modifications "bleed" over to unintended cookies if the CookieOptions is reused for multiple cookies, for example if it's stored in a static field in a controller.

Expected Behavior

Neither the documentation for CookieOptions nor for the CookiePolicyMiddleware recommend against reusing CookieOptions. To prevent this problem, I would either:

  1. Warn against reusing CookieOptions in the documentation, if they are supposed to be instantiated anew for each cookie, or
  2. Change CookiePolicyMiddleware so it clones the passed-in CookieOptions object before making modifications to it. (Luckily, there's already a CookieOptions(CookieOptions) constructor for this.)

Steps To Reproduce

Create a new empty ASP.NET Core app and replace the content with:

using Microsoft.AspNetCore.CookiePolicy;

var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();

// Cookies should always be secure. Let's use this CookieOptions object whenever appending a cookie to ensure that.
var cookieOptions = new CookieOptions
{
    Secure = true,
    IsEssential = true
};

app.UseWhen(context => context.Request.Path.StartsWithSegments("/http-only"),
    applicationBuilder =>
        applicationBuilder.UseCookiePolicy(new CookiePolicyOptions { HttpOnly = HttpOnlyPolicy.Always }));

app.MapGet("/", context =>
{
    context.Response.Cookies.Append("should-not-be", "http-only", cookieOptions);
    return context.Response.WriteAsync("non-http-only cookie set");
});
app.MapGet("/http-only", context =>
{
    context.Response.Cookies.Append("should-be", "http-only", cookieOptions);
    return context.Response.WriteAsync("http-only cookie set");
});

app.Run();

Run it, then visit /, /http-only, then / again. Using the browser developer tools, notice how the should-not-be cookie is inadvertently set as HttpOnly on the second visit to /.

Exceptions (if any)

No response

.NET Version

No response

Anything else?

This caused a production incident for us when we tried using the CookiePolicyMiddleware to automatically set the new Partitioned attribute (part of the CHIPS proposal which allows adapting the the Google Chrome phase-out of third-party cookies) on all cookies for a subset of test users using the CookiePolicyOptions.OnAppendCookie delegate, and it ended up applying to all users for a specific cookie set from a controller which reused the CookieOptions.

Metadata

Metadata

Assignees

No one assigned

    Labels

    DocsThis issue tracks updating documentationarea-middlewareIncludes: URL rewrite, redirect, response cache/compression, session, and other general middlewares

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions