From 74c424dc4cb4bfd6bbb81f2df4b5f53022fb55c1 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Wed, 5 Nov 2025 09:57:26 +0000 Subject: [PATCH] Fixes, add secure query --- .../lib/semmle/go/security/SecureCookies.qll | 55 ++++++++++++++----- .../CWE-1004/CookieWithoutHttpOnly.ql | 19 +++---- .../Security/CWE-614/CookieWithoutSecure.ql | 18 ++++++ 3 files changed, 69 insertions(+), 23 deletions(-) create mode 100644 go/ql/src/Security/CWE-614/CookieWithoutSecure.ql diff --git a/go/ql/lib/semmle/go/security/SecureCookies.qll b/go/ql/lib/semmle/go/security/SecureCookies.qll index 654acc81569..f700c030337 100644 --- a/go/ql/lib/semmle/go/security/SecureCookies.qll +++ b/go/ql/lib/semmle/go/security/SecureCookies.qll @@ -27,10 +27,12 @@ private module SensitiveCookieNameConfig implements DataFlow::ConfigSig { } /** Tracks flow from sensitive names to HTTP cookie writes. */ -module SensitiveCookieNameFlow = DataFlow::Global; +module SensitiveCookieNameFlow = TaintTracking::Global; private module BooleanCookieSecureConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { exists(source.asExpr().getBoolValue()) } + predicate isSource(DataFlow::Node source) { + source.getType().getUnderlyingType() instanceof BoolType + } predicate isSink(DataFlow::Node sink) { exists(Http::CookieWrite cw | sink = cw.getSecure()) } @@ -39,11 +41,13 @@ private module BooleanCookieSecureConfig implements DataFlow::ConfigSig { } } -/** Tracks flow from boolean expressions to the `Secure` attribute HTTP cookie writes. */ -module BooleanCookieSecureFlow = DataFlow::Global; +/** Tracks flow from boolean expressions to the `Secure` attribute of HTTP cookie writes. */ +module BooleanCookieSecureFlow = TaintTracking::Global; private module BooleanCookieHttpOnlyConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { exists(source.asExpr().getBoolValue()) } + predicate isSource(DataFlow::Node source) { + source.getType().getUnderlyingType() instanceof BoolType + } predicate isSink(DataFlow::Node sink) { exists(Http::CookieWrite cw | sink = cw.getHttpOnly()) } @@ -52,28 +56,53 @@ private module BooleanCookieHttpOnlyConfig implements DataFlow::ConfigSig { } } -/** Tracks flow from boolean expressions to the `HttpOnly` attribute HTTP cookie writes. */ -module BooleanCookieHttpOnlyFlow = DataFlow::Global; +/** Tracks flow from boolean expressions to the `HttpOnly` attribute of HTTP cookie writes. */ +module BooleanCookieHttpOnlyFlow = TaintTracking::Global; +/** Holds if `cw` has the `Secure` attribute left at its default value of `false`. */ predicate isInsecureDefault(Http::CookieWrite cw) { not BooleanCookieSecureFlow::flow(_, cw.getSecure()) } -predicate isNonHttpOnlyDefault(Http::CookieWrite cw) { - not BooleanCookieHttpOnlyFlow::flow(_, cw.getHttpOnly()) -} - +/** Holds if `cw` has the `Secure` attribute explicitly set to `false`, from the expression `boolFalse`. */ predicate isInsecureDirect(Http::CookieWrite cw, Expr boolFalse) { BooleanCookieSecureFlow::flow(DataFlow::exprNode(boolFalse), cw.getSecure()) and boolFalse.getBoolValue() = false } +/** Holds if `cw` has the `Secure` attribute set to `false`, either explicitly or by default. */ +predicate isInsecureCookie(Http::CookieWrite cw) { + isInsecureDefault(cw) or + isInsecureDirect(cw, _) +} + +/** Holds if `cw` has the `HttpOnly` attribute left at its default value of `false`. */ +predicate isNonHttpOnlyDefault(Http::CookieWrite cw) { + not BooleanCookieHttpOnlyFlow::flow(_, cw.getHttpOnly()) +} + +/** Holds if `cw` has the `HttpOnly` attribute explicitly set to `false`, from the expression `boolFalse`. */ predicate isNonHttpOnlyDirect(Http::CookieWrite cw, Expr boolFalse) { BooleanCookieHttpOnlyFlow::flow(DataFlow::exprNode(boolFalse), cw.getHttpOnly()) and boolFalse.getBoolValue() = false } -predicate isSensitiveCookie(Http::CookieWrite cw, Expr nameExpr, string name) { - SensitiveCookieNameFlow::flow(DataFlow::exprNode(nameExpr), cw.getName()) and +/** Holds if `cw` has the `HttpOnly` attribute set to `false`, either explicitly or by default. */ +predicate isNonHttpOnlyCookie(Http::CookieWrite cw) { + isNonHttpOnlyDefault(cw) or + isNonHttpOnlyDirect(cw, _) +} + +/** + * Holds if `cw` has the sensitive name `name`, from the expression `nameExpr`. + * `source` and `sink` represent the data flow path from the sensitive name expression to the cookie write. + */ +predicate isSensitiveCookie( + Http::CookieWrite cw, Expr nameExpr, string name, SensitiveCookieNameFlow::PathNode source, + SensitiveCookieNameFlow::PathNode sink +) { + SensitiveCookieNameFlow::flowPath(source, sink) and + source.getNode().asExpr() = nameExpr and + sink.getNode() = cw.getName() and isSensitiveExpr(nameExpr, name) } diff --git a/go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql b/go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql index e61fd652680..27f86d23dd2 100644 --- a/go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql +++ b/go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql @@ -4,25 +4,24 @@ * malicious JavaScript to steal it in case of XSS vulnerability. Always set * 'HttpOnly' to 'true' to authentication related cookie to make it * not accessible by JavaScript. - * @kind problem + * @kind path-problem * @problem.severity warning * @precision high * @id go/cookie-httponly-not-set * @tags security - * experimental * external/cwe/cwe-1004 */ import go import semmle.go.security.SecureCookies import semmle.go.concepts.HTTP +import SensitiveCookieNameFlow::PathGraph -from Http::CookieWrite cw, Expr sensitiveNameExpr, string name +from + Http::CookieWrite cw, Expr sensitiveNameExpr, string name, + SensitiveCookieNameFlow::PathNode source, SensitiveCookieNameFlow::PathNode sink where - isSensitiveCookie(cw, sensitiveNameExpr, name) and - ( - isNonHttpOnlyDefault(cw) - or - isNonHttpOnlyDirect(cw, _) - ) -select cw, "Sensitive cookie $@ does not set HttpOnly to true", sensitiveNameExpr, name + isSensitiveCookie(cw, sensitiveNameExpr, name, source, sink) and + isNonHttpOnlyCookie(cw) +select cw, source, sink, "Sensitive cookie $@ does not set HttpOnly attribute to true.", + sensitiveNameExpr, name diff --git a/go/ql/src/Security/CWE-614/CookieWithoutSecure.ql b/go/ql/src/Security/CWE-614/CookieWithoutSecure.ql new file mode 100644 index 00000000000..635d2113f8d --- /dev/null +++ b/go/ql/src/Security/CWE-614/CookieWithoutSecure.ql @@ -0,0 +1,18 @@ +/** + * @name 'Secure' attribute is not set to true + * @description todo + * @kind problem + * @problem.severity warning + * @precision high + * @id go/cookie-secure-not-set + * @tags security + * external/cwe/cwe-1004 + */ + +import go +import semmle.go.security.SecureCookies +import semmle.go.concepts.HTTP + +from Http::CookieWrite cw +where isInsecureCookie(cw) +select cw, "Cookie does not set Secure attribute to true"