diff --git a/go/ql/lib/semmle/go/security/SecureCookies.qll b/go/ql/lib/semmle/go/security/CookieWithoutHttpOnly.qll similarity index 69% rename from go/ql/lib/semmle/go/security/SecureCookies.qll rename to go/ql/lib/semmle/go/security/CookieWithoutHttpOnly.qll index 5904adbed39..915a58ef369 100644 --- a/go/ql/lib/semmle/go/security/SecureCookies.qll +++ b/go/ql/lib/semmle/go/security/CookieWithoutHttpOnly.qll @@ -1,4 +1,4 @@ -/** Provides classes and predicates for identifying HTTP cookies with insecure attributes. */ +/** Provides classes and predicates for identifying HTTP cookies without the `HttpOnly` attribute. */ import go import semmle.go.concepts.HTTP @@ -31,21 +31,6 @@ private module SensitiveCookieNameConfig implements DataFlow::ConfigSig { /** Tracks flow from sensitive names to HTTP cookie writes. */ module SensitiveCookieNameFlow = TaintTracking::Global; -private module BooleanCookieSecureConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { - source.getType().getUnderlyingType() instanceof BoolType - } - - predicate isSink(DataFlow::Node sink) { exists(Http::CookieWrite cw | sink = cw.getSecure()) } - - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { - exists(Http::CookieOptionWrite co | co.getSecure() = pred and co.getCookieOutput() = succ) - } -} - -/** 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) { source.getType().getUnderlyingType() instanceof BoolType @@ -61,23 +46,6 @@ private module BooleanCookieHttpOnlyConfig implements DataFlow::ConfigSig { /** 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()) -} - -/** 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()) diff --git a/go/ql/lib/semmle/go/security/CookieWithoutSecure.qll b/go/ql/lib/semmle/go/security/CookieWithoutSecure.qll new file mode 100644 index 00000000000..86d65ca5c0f --- /dev/null +++ b/go/ql/lib/semmle/go/security/CookieWithoutSecure.qll @@ -0,0 +1,37 @@ +/** Provides classes and predicates for identifying HTTP cookies without the `Secure` attribute. */ + +import go +import semmle.go.concepts.HTTP +import semmle.go.dataflow.DataFlow + +private module BooleanCookieSecureConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source.getType().getUnderlyingType() instanceof BoolType + } + + predicate isSink(DataFlow::Node sink) { exists(Http::CookieWrite cw | sink = cw.getSecure()) } + + predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(Http::CookieOptionWrite co | co.getSecure() = pred and co.getCookieOutput() = succ) + } +} + +/** Tracks flow from boolean expressions to the `Secure` attribute of HTTP cookie writes. */ +module BooleanCookieSecureFlow = 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()) +} + +/** 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, _) +} diff --git a/go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql b/go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql index 59881da8aa7..8d11d8c40a6 100644 --- a/go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql +++ b/go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql @@ -12,14 +12,14 @@ */ import go -import semmle.go.security.SecureCookies +import semmle.go.security.CookieWithoutHttpOnly import SensitiveCookieNameFlow::PathGraph from - Http::CookieWrite cw, Expr sensitiveNameExpr, string name, - SensitiveCookieNameFlow::PathNode source, SensitiveCookieNameFlow::PathNode sink + Http::CookieWrite cw, string name, SensitiveCookieNameFlow::PathNode source, + SensitiveCookieNameFlow::PathNode sink where - isSensitiveCookie(cw, sensitiveNameExpr, name, source, sink) and + isSensitiveCookie(cw, name, source, sink) and isNonHttpOnlyCookie(cw) -select cw, source, sink, "Sensitive cookie $@ does not set HttpOnly attribute to true.", - sensitiveNameExpr, name +select cw, source, sink, "Sensitive cookie $@ does not set HttpOnly attribute to true.", source, + name diff --git a/go/ql/src/Security/CWE-614/CookieWithoutSecure.ql b/go/ql/src/Security/CWE-614/CookieWithoutSecure.ql index 9ff157cbbbb..7c3c4ed1dd3 100644 --- a/go/ql/src/Security/CWE-614/CookieWithoutSecure.ql +++ b/go/ql/src/Security/CWE-614/CookieWithoutSecure.ql @@ -5,14 +5,14 @@ * @kind problem * @problem.severity warning * @precision high - * @security-severity 5.0 + * @security-severity 4.0 * @id go/cookie-secure-not-set * @tags security * external/cwe/cwe-614 */ import go -import semmle.go.security.SecureCookies +import semmle.go.security.CookieWithoutSecure from Http::CookieWrite cw where isInsecureCookie(cw)