Skip to content

Conversation

@joefarebrother
Copy link
Contributor

Promotes the cs/web/cookie-secure-not-set and cs/web/cookie-httponly-not-set queries from experimental.

@joefarebrother joefarebrother requested a review from a team as a code owner October 24, 2025 15:09
Copilot AI review requested due to automatic review settings October 24, 2025 15:09
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR promotes the cs/web/cookie-secure-not-set and cs/web/cookie-httponly-not-set queries from experimental to the main query pack, making them available for standard security analysis.

Key changes:

  • Moved cookie security queries from experimental to main Security Features directory
  • Reorganized test structure to align with promoted query locations
  • Updated query metadata and documentation for production readiness

Reviewed Changes

Copilot reviewed 111 out of 137 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
csharp/ql/src/Security Features/CWE-614/CookieWithoutSecure.ql New production query for detecting cookies without Secure attribute
csharp/ql/src/Security Features/CWE-1004/CookieWithoutHttpOnly.ql New production query for detecting cookies without HttpOnly attribute
csharp/ql/lib/semmle/code/csharp/security/auth/SecureCookies.qll Updated library removing deprecated annotations and improving documentation
csharp/ql/test/query-tests/Security Features/CWE-614/InsecureCookie/ New test structure for Secure attribute query
csharp/ql/test/query-tests/Security Features/CWE-1004/HttpOnlyCookie/ New test structure for HttpOnly attribute query
csharp/ql/test/experimental/Security Features/CWE-614/ Removed experimental tests (moved to main)
csharp/ql/test/experimental/Security Features/CWE-1004/ Removed experimental tests (moved to main)
csharp/ql/src/experimental/Security Features/CWE-614/ Removed experimental query sources
csharp/ql/src/experimental/Security Features/CWE-1004/ Removed experimental query sources
csharp/ql/src/change-notes/2025-10-24-insecure-cookie-query-promote.md Release note documenting query promotion

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -1,13 +1,12 @@
/**
* Provides classes and predicates for detecting insecure cookies.
* Definitions for detecting insecure and non-httponly cookies.
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description should use consistent terminology. Change 'non-httponly' to 'non-HttpOnly' to match the casing used throughout the codebase and in the HttpOnly property name.

Suggested change
* Definitions for detecting insecure and non-httponly cookies.
* Definitions for detecting insecure and non-HttpOnly cookies.

Copilot uses AI. Check for mistakes.
<references>

<li>ASP.Net Core docs: <a href="https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.cookieoptions.httponly">CookieOptions.HttpOnly Property</a>.</li>
<li>MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie">Set-Cookie</a> Header</li>.
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the extra period after the closing </li> tag - the period should be inside the tag or removed entirely.

Suggested change
<li>MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie">Set-Cookie</a> Header</li>.
<li>MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie">Set-Cookie</a> Header.</li>

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Contributor

QHelp previews:

csharp/ql/src/Security Features/CWE-1004/CookieWithoutHttpOnly.qhelp

Cookie 'HttpOnly' attribute is not set to true

Cookies without the HttpOnly flag set are accessible to client-side scripts such as JavaScript running in the same origin. In case of a Cross-Site Scripting (XSS) vulnerability, the cookie can be stolen by a malicious script. If a sensitive cookie does not need to be accessed directly by client-side JS, the HttpOnly flag should be set.

Recommendation

Set the HttpOnly flag to true for authentication cookies to ensure they are not accessible to client-side scripts.

When using ASP.NET Core, CookiePolicyOptions can be used to set a default policy for cookies. When using ASP.NET Web Forms, a default may also be configured in the Web.config file, using the httpOnlyCookies attribute of the the <httpCookies> element.

Example

In the example below, Microsoft.AspNetCore.Http.CookieOptions.HttpOnly is set to true.

class MyController : Controller
{
    void Login()
    {
        var cookieOptions = new Microsoft.AspNetCore.Http.CookieOptions() { HttpOnly = true };
        Response.Cookies.Append("auth", "secret", cookieOptions);
    }
}

In the following example, CookiePolicyOptions are set programmatically to configure defaults.

public class Startup
{
    // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
    public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
    {
        app.UseCookiePolicy(new CookiePolicyOptions()
        {
            Secure = Microsoft.AspNetCore.Http.CookieSecurePolicy.Always,
            HttpOnly = Microsoft.AspNetCore.CookiePolicy.HttpOnlyPolicy.Always
        });
    }
}

In the example below, System.Web.HttpCookie.HttpOnly is set to true.

class MyController : Controller
{
    void Login()
    {
        var cookie = new System.Web.HttpCookie("cookieName") { HttpOnly = true };
    }
}

In the example below, the httpOnlyCookies attribute is set to true in the Web.config file.

<?xml version="1.0" encoding="utf-8" ?>
<configuration>
  <system.web>
    <httpCookies httpOnlyCookies="true"/>
  </system.web>
</configuration>

References

csharp/ql/src/Security Features/CWE-614/CookieWithoutSecure.qhelp

Cookie 'Secure' attribute is not set to true

Cookies without the Secure flag set may be transmitted using HTTP instead of HTTPS. This leaves them vulnerable to being read by a third party attacker. If a sensitive cookie such as a session key is intercepted this way, it would allow the attacker to perform actions on a user's behalf.

Recommendation

When using ASP.NET Core, ensure cookies have the secure flag set by setting Microsoft.AspNetCore.Http.CookieOptions.Secure to true, or using CookiePolicyOptions to set a default security policy.

When using ASP.NET Web Forms, cookies can be configured as secure by default in the Web.config file, setting the requireSSL attribute to true in the forms or httpCookies element. Cookies may also be set to be secure programmatically by setting the System.Web.HttpCookie.Secure attribute to true.

Example

In the example below, Microsoft.AspNetCore.Http.CookieOptions.Secure is set to true.

class MyController : Controller
{
    void Login()
    {
        var cookieOptions = new Microsoft.AspNetCore.Http.CookieOptions() { Secure = true };
        Response.Cookies.Append("auth", "secret", cookieOptions);
    }
}

In the following example, CookiePolicyOptions are set programmatically to configure defaults.

public class Startup
{
    // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
    public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
    {
        app.UseCookiePolicy(new CookiePolicyOptions()
        {
            Secure = Microsoft.AspNetCore.Http.CookieSecurePolicy.Always,
            HttpOnly = Microsoft.AspNetCore.CookiePolicy.HttpOnlyPolicy.Always
        });
    }
}

In the example below System.Web.HttpCookie.Secure is set to true programmatically.

class MyController : Controller
{
    void Login()
    {
        var cookie = new System.Web.HttpCookie("cookieName") { Secure = true };
    }
}

In the example below, the requireSSL attribute is set to true in the forms element of the Web.config file.

<?xml version="1.0" encoding="utf-8" ?>
<configuration>
  <system.web>
    <authentication>
      <forms
        requireSSL="true"
        ... />
    </authentication>
    <httpCookies
        requireSSL="true"
        ... />
  </system.web>
</configuration>

References

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant