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

go/ql/lib/semmle/go/concepts/HTTP.qll

@@ -380,4 +380,96 @@
     /** Gets a node that is used in a check that is tested before this handler is run. */
     predicate guardedBy(DataFlow::Node check) { super.guardedBy(check) }
   }
+
+  /** Provides a class for modelling HTTP response cookie writes. */
+  module CookieWrite {
+    /**
+     * An write of an HTTP Cookie to an HTTP response.
+     *
+     * Extend this class to model new APIs. If you want to refine existing API models,
+     * extend `HTTP::CookieWrite` instead.
+     */
+    abstract class Range extends DataFlow::Node {
+      /** Gets the name of the cookie written. */
+      abstract DataFlow::Node getName();
+
+      /** Gets the value of the cookie written. */
+      abstract DataFlow::Node getValue();
+
+      /** Gets the `Secure` attribute of the cookie written. */
+      abstract DataFlow::Node getSecure();
+
+      /** Gets the `HttpOnly` attribute of the cookie written. */
+      abstract DataFlow::Node getHttpOnly();
+    }
+  }
+
+  /**
+   * An write of an HTTP Cookie to an HTTP response.
+   *
+   * Extend this class to refine existing API models. If you want to model new APIs,
+   * extend `HTTP::CookieWrite::Range` instead.
+   */
+  class CookieWrite extends DataFlow::Node instanceof CookieWrite::Range {
+    /** Gets the name of the cookie written. */
+    DataFlow::Node getName() { result = super.getName() }
+
+    /** Gets the value of the cookie written. */
+    DataFlow::Node getValue() { result = super.getValue() }
+
+    /** Gets the `Secure` attribute of the cookie written. */
+    DataFlow::Node getSecure() { result = super.getSecure() }
+
+    /** Gets the `HttpOnly` attribute of the cookie written. */
+    DataFlow::Node getHttpOnly() { result = super.getHttpOnly() }
+  }
+
+  /** Provides a class for modelling the options of an HTTP cookie. */
+  module CookieOptions {
+    /**
+     * An HTTP Cookie object.
+     *
+     * Extend this class to model new APIs. If you want to refine existing API models,
+     * extend `HTTP::CookieOptions` instead.
+     */
+    abstract class Range extends DataFlow::Node {
+      /** Gets the node representing the cookie object for the options being set. */
+      abstract DataFlow::Node getCookieOutput();
+
+      /** Gets the name of the cookie represented. */
+      abstract DataFlow::Node getName();
+
+      /** Gets the value of the cookie represented. */
+      abstract DataFlow::Node getValue();
+
+      /** Gets the `Secure` attribute of the cookie represented. */
+      abstract DataFlow::Node getSecure();
+
+      /** Gets the `HttpOnly` attribute of the cookie represented. */
+      abstract DataFlow::Node getHttpOnly();
+    }
+  }
+
+  /**
+   * An HTTP Cookie.
+   *
+   * Extend this class to refine existing API models. If you want to model new APIs,
+   * extend `HTTP::CookieOptions::Range` instead.
+   */
+  class CookieOptions extends DataFlow::Node instanceof CookieOptions::Range {
+    /** Gets the node representing the cookie object for the options being set. */
+    DataFlow::Node getCookieOutput() { result = super.getCookieOutput() }
+
+    /** Gets the name of the cookie represented. */
+    DataFlow::Node getName() { result = super.getName() }
+
+    /** Gets the value of the cookie represented. */
+    DataFlow::Node getValue() { result = super.getValue() }
+
+    /** Gets the `Secure` attribute of the cookie represented. */
+    DataFlow::Node getSecure() { result = super.getSecure() }
+
+    /** Gets the `HttpOnly` attribute of the cookie represented. */
+    DataFlow::Node getHttpOnly() { result = super.getHttpOnly() }
+  }
 }

go/ql/lib/semmle/go/frameworks/Gin.qll

@@ -0,0 +1,24 @@
+/**
+ * Provides classes for modeling the `github.com/gin-gonic/gin` package.
+ */
+
+import go
+import semmle.go.concepts.HTTP
+
+/** Provides models for the `gin-gonic/gin` package. */
+module Gin {
+  /** Gets the package name `github.com/gin-gonic/gin`. */
+  string packagePath() { result = package("github.com/gin-gonic/gin", "") }
+
+  private class GinCookieWrite extends Http::CookieWrite::Range, DataFlow::MethodCallNode {
+    GinCookieWrite() { this.getTarget().hasQualifiedName(packagePath(), "Context", "SetCookie") }
+
+    override DataFlow::Node getName() { result = this.getArgument(0) }
+
+    override DataFlow::Node getValue() { result = this.getArgument(1) }
+
+    override DataFlow::Node getSecure() { result = this.getArgument(5) }
+
+    override DataFlow::Node getHttpOnly() { result = this.getArgument(6) }
+  }
+}

go/ql/lib/semmle/go/frameworks/stdlib/NetHttp.qll

@@ -293,4 +293,38 @@
 
     override DataFlow::Node getAPathArgument() { result = this.getArgument(2) }
   }
+
+  class CookieWrite extends Http::CookieWrite::Range, DataFlow::CallNode {
+    CookieWrite() { this.getTarget().hasQualifiedName(package("net/http", ""), "SetCookie") }
+
+    override DataFlow::Node getName() { result = this.getArgument(1) }
+
+    override DataFlow::Node getValue() { result = this.getArgument(1) }
+
+    override DataFlow::Node getSecure() { result = this.getArgument(1) }
+
+    override DataFlow::Node getHttpOnly() { result = this.getArgument(1) }
+  }
+
+  class CookieFieldWrite extends Http::CookieOptions::Range {
+    Write w;
+    Field f;
+    DataFlow::Node written;
+    string fieldName;
+
+    CookieFieldWrite() {
+      f.hasQualifiedName(package("net/http", ""), "Cookie", fieldName) and
+      w.writesField(this, f, written)
+    }
+
+    override DataFlow::Node getCookieOutput() { result = this }
+
+    override DataFlow::Node getName() { fieldName = "Name" and result = written }
+
+    override DataFlow::Node getValue() { fieldName = "Value" and result = written }
+
+    override DataFlow::Node getSecure() { fieldName = "Secure" and result = written }
+
+    override DataFlow::Node getHttpOnly() { fieldName = "HttpOnly" and result = written }
+  }
 }

go/ql/lib/semmle/go/security/SecureCookies.qll

@@ -0,0 +1,108 @@
+/** Provides classes and predicates for identifying HTTP cookies with insecure attributes. */
+
+import go
+import semmle.go.concepts.HTTP
+import semmle.go.dataflow.DataFlow
+
+/**
+ * Holds if the expression or its value has a sensitive name
+ */
+private predicate isSensitiveExpr(Expr expr, string val) {
+  (
+    val = expr.getStringValue() or
+    val = expr.(Name).getTarget().getName()
+  ) and
+  val.regexpMatch("(?i).*(session|login|token|user|auth|credential).*") and
+  not val.regexpMatch("(?i).*(xsrf|csrf|forgery).*")
+}
+
+private module SensitiveCookieNameConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { isSensitiveExpr(source.asExpr(), _) }
+
+  predicate isSink(DataFlow::Node sink) { exists(Http::CookieWrite cw | sink = cw.getName()) }
+
+  predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(Http::CookieOptions co | co.getName() = pred and co.getCookieOutput() = succ)
+  }
+}
+
+/** Tracks flow from sensitive names to HTTP cookie writes. */
+module SensitiveCookieNameFlow = TaintTracking::Global<SensitiveCookieNameConfig>;
+
+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::CookieOptions 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<BooleanCookieSecureConfig>;
+
+private module BooleanCookieHttpOnlyConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source.getType().getUnderlyingType() instanceof BoolType
+  }
+
+  predicate isSink(DataFlow::Node sink) { exists(Http::CookieWrite cw | sink = cw.getHttpOnly()) }
+
+  predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(Http::CookieOptions co | co.getHttpOnly() = pred and co.getCookieOutput() = succ)
+  }
+}
+
+/** Tracks flow from boolean expressions to the `HttpOnly` attribute of HTTP cookie writes. */
+module BooleanCookieHttpOnlyFlow = TaintTracking::Global<BooleanCookieHttpOnlyConfig>;
+
+/** 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())
+}
+
+/** 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
+}
+
+/** 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)
+}

go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql

@@ -0,0 +1,27 @@
+/**
+ * @name 'HttpOnly' attribute is not set to true
+ * @description Omitting the 'HttpOnly' attribute for security sensitive data allows
+ *              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 path-problem
+ * @problem.severity warning
+ * @precision high
+ * @id go/cookie-httponly-not-set
+ * @tags security
+ *       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,
+  SensitiveCookieNameFlow::PathNode source, SensitiveCookieNameFlow::PathNode sink
+where
+  isSensitiveCookie(cw, sensitiveNameExpr, name, source, sink) and
+  isNonHttpOnlyCookie(cw)
+select cw, source, sink, "Sensitive cookie $@ does not set HttpOnly attribute to true.",
+  sensitiveNameExpr, name

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"