Go: Promote non-httponly cookie query, and add insecure cookie query

[joefarebrother]

Promotes go/cookie-httponly-not-set from experimental, and adds go/cookie-secure-not-set query.

[github-actions]

QHelp previews:

go/ql/src/Security/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.

Example

In the following example, in the case marked BAD, the HttpOnly flag is not set, so the default value of false is used. In the case marked GOOD, the HttpOnly flag is set to true.

package main

import (
	"net/http"
)

func handlerBad(w http.ResponseWriter, r *http.Request) {
	c := http.Cookie{
		Name:  "session",
		Value: "secret",
	}
	http.SetCookie(w, &c) // BAD: The HttpOnly flag is set to false by default.
}

func handlerGood(w http.ResponseWriter, r *http.Request) {
	c := http.Cookie{
		Name:     "session",
		Value:    "secret",
		HttpOnly: true,
	}
	http.SetCookie(w, &c) // GOOD: The HttpOnly flag is set to true.
}

References

• MDN: Set-Cookie Header.
• PortSwigger: Cookie without HttpOnly flag set
• Common Weakness Enumeration: CWE-1004.

go/ql/src/Security/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

Set the Secure flag to true to ensure cookies are only transmitted over secure HTTPS connections.

Example

In the following example, in the case marked BAD, the Secure flag is set to false by default. In the case marked GOOD, the Secure flag is set to true.

package main

import (
	"net/http"
)

func handlerBad(w http.ResponseWriter, r *http.Request) {
	c := http.Cookie{
		Name:  "session",
		Value: "secret",
	}
	http.SetCookie(w, &c) // BAD: The Secure flag is set to false by default.
}

func handlerGood(w http.ResponseWriter, r *http.Request) {
	c := http.Cookie{
		Name:   "session",
		Value:  "secret",
		Secure: true,
	}
	http.SetCookie(w, &c) // GOOD: The Secure flag is set to true.
}

References

• MDN: Set-Cookie Header.
• Detectify: Cookie lack Secure flag.
• PortSwigger: TLS cookie without secure flag set.
• Common Weakness Enumeration: CWE-614.

[Copilot]

Pull Request Overview

This PR promotes an experimental query for detecting cookies without the HttpOnly flag and introduces a new query for detecting cookies without the Secure flag. The changes include:

• Migration of the CookieWithoutHttpOnly query from experimental to the main security query pack
• Introduction of a new CookieWithoutSecure query
• New shared library code (SecureCookies.qll) for modeling HTTP cookie security attributes
• Updates to framework models for net/http and gin-gonic/gin

Reviewed Changes

Copilot reviewed 31 out of 42 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql	New query for detecting cookies without HttpOnly flag
go/ql/src/Security/CWE-614/CookieWithoutSecure.ql	New query for detecting cookies without Secure flag
go/ql/lib/semmle/go/security/SecureCookies.qll	Shared library for cookie security analysis
go/ql/lib/semmle/go/concepts/HTTP.qll	Added CookieWrite and CookieOptions concepts
go/ql/lib/semmle/go/frameworks/NetHttp.qll	Added cookie write models for net/http
go/ql/lib/semmle/go/frameworks/Gin.qll	New framework models for gin-gonic/gin
go/ql/src/change-notes/2025-11-10-inseucre-cookie.md	Change notes documenting the updates
go/ql/src/experimental/CWE-1004/*	Removed experimental query files
go/ql/test/query-tests/Security/CWE-614/*	Test files for CookieWithoutSecure
go/ql/test/query-tests/Security/CWE-1004/*	Test files for CookieWithoutHttpOnly

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[owen-mc]

I love the new concepts. I think they make the structure quite clean, and allow us to add other libraries easily in future.

I've only done a partial review so far. It will be easier to continue once these points have been addressed.

I note that you've removed modelling and tests for github.com/gorilla/sessions. Was that deliberate? It seems like a popular enough library, and it's only a bit of extra modelling and some tests that have already been written.

[joefarebrother]

Addressed comments
I can re-add the gorilla/sessions models; they were initially ommited due to a slight complication in which there isn't a sensitive name expression to alert for as the source; however it can be handled by alerting for the cookie constructor with a name of something like "Gorilla session".

[owen-mc]

Thanks for applying all those changes. On second thought, maybe it's too much work to add modeling for Gorilla. You can leave it for now. But please remove the stubs for it, and mentions of it from the go.mod and modules.txt files.

My only slight worry is that performance might be bad in certain situations. Let me explain. For the insecure cookie query, to get the best performance I think you would have two data flow configs: one from getCookieOutput() of CookieOptionsWrite for getSecure exists to CookieWrite, and one from any boolean typed node with getBooleanValue() = false to the getSecure() of a CookieOptionsWrite (for extra performance you would also require that the getCookieOutput() of it is a source a path for the first data flow config. By using just one config you've made is simpler, but you could end up with lots of sources along the same path, and hence lots of paths. Maybe that's okay performance-wise. But the DCA run seems fine, so no need to change anything - we can always look into it in future if needed.

[owen-mc]

Have you checked how autofix works on these queries?

[owen-mc]

Looks good, as long as autofix is good. You also need to add it to a list of queries to generate autofixes for, I believe, but that is in a different repo so not part of this PR.

[joefarebrother]

Autofix results look good.