Support string expressions

Author: edvraa

javascript/ql/src/experimental/semmle/javascript/security/InsecureCookie.qll

@@ -45,31 +45,31 @@ module Cookie {
      * Holds if the cookie is authentication sensitive and lacks HttpOnly.
      */
     abstract predicate isAuthNotHttpOnly();
+  }
 
-    /**
-     * Holds if the expression is a variable with a sensitive name.
-     */
-    predicate isAuthVariable(DataFlow::Node expr) {
-      exists(string val |
-        (
-          val = expr.getStringValue() or
-          val = expr.asExpr().(VarAccess).getName() or
-          val = expr.(DataFlow::PropRead).getPropertyName()
-        ) and
-        regexpMatchAuth(val)
-      )
-      or
-      isAuthVariable(expr.getAPredecessor())
-    }
+  /**
+   * Holds if the expression is a variable with a sensitive name.
+   */
+  private predicate isAuthVariable(DataFlow::Node expr) {
+    exists(string val |
+      (
+        val = expr.getStringValue() or
+        val = expr.asExpr().(VarAccess).getName() or
+        val = expr.(DataFlow::PropRead).getPropertyName()
+      ) and
+      regexpMatchAuth(val)
+    )
+    or
+    isAuthVariable(expr.getAPredecessor())
+  }
 
-    /**
-     * Holds if the string contains sensitive auth keyword, but not antiforgery token.
-     */
-    bindingset[val]
-    predicate regexpMatchAuth(string val) {
-      val.regexpMatch("(?i).*(session|login|token|user|auth|credential).*") and
-      not val.regexpMatch("(?i).*(xsrf|csrf|forgery).*")
-    }
+  /**
+   * Holds if the string contains sensitive auth keyword, but not antiforgery token.
+   */
+  bindingset[val]
+  private predicate regexpMatchAuth(string val) {
+    val.regexpMatch("(?i).*(session|login|token|user|auth|credential).*") and
+    not val.regexpMatch("(?i).*(xsrf|csrf|forgery).*")
   }
 
   /**
@@ -168,6 +168,26 @@ module Cookie {
     }
   }
 
+  private class AttributeToSetCookieHeaderTrackingConfig extends TaintTracking::Configuration {
+    AttributeToSetCookieHeaderTrackingConfig() { this = "AttributeToSetCookieHeaderTrackingConfig" }
+
+    override predicate isSource(DataFlow::Node source) {
+      exists(string s | source.mayHaveStringValue(s))
+    }
+
+    override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof TemplateLiteral }
+  }
+
+  private class SensitiveNameToSetCookieHeaderTrackingConfig extends TaintTracking::Configuration {
+    SensitiveNameToSetCookieHeaderTrackingConfig() {
+      this = "SensitiveNameToSetCookieHeaderTrackingConfig"
+    }
+
+    override predicate isSource(DataFlow::Node source) { isAuthVariable(source) }
+
+    override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof TemplateLiteral }
+  }
+
   /**
    * A cookie set using `Set-Cookie` header of an `HTTP` response.
    * (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie).
@@ -197,7 +217,7 @@ module Cookie {
      * A cookie is httpOnly if the `httpOnly` flag is specified in the cookie definition.
      * The default is `false`.
      */
-    override predicate isHttpOnly() { allHaveCookieAttribute("httponly") }
+    override predicate isHttpOnly() { allHaveCookieAttribute(httpOnlyFlag()) }
 
     /**
      * The predicate holds only if all elements have the specified attribute.
@@ -209,6 +229,14 @@ module Cookie {
           n.mayHaveStringValue(s) and
           hasCookieAttribute(s, attribute)
         )
+        or
+        exists(AttributeToSetCookieHeaderTrackingConfig cfg, DataFlow::Node source |
+          cfg.hasFlow(source, n) and
+          exists(string attr |
+            source.mayHaveStringValue(attr) and
+            attr.regexpMatch("(?i).*\\b" + attribute + "\\b.*")
+          )
+        )
       )
     }
 
@@ -221,10 +249,19 @@ module Cookie {
         exists(string s |
           n.mayHaveStringValue(s) and
           (
-            not hasCookieAttribute(s, "httponly") and
+            not hasCookieAttribute(s, httpOnlyFlag()) and
             regexpMatchAuth(getCookieName(s))
           )
         )
+        or
+        not exists(AttributeToSetCookieHeaderTrackingConfig cfg, DataFlow::Node source |
+          cfg.hasFlow(source, n) and
+          exists(string attr |
+            source.mayHaveStringValue(attr) and
+            attr.regexpMatch("(?i).*\\b" + httpOnlyFlag() + "\\b.*")
+          )
+        ) and
+        exists(SensitiveNameToSetCookieHeaderTrackingConfig cfg | cfg.hasFlow(_, n))
       )
     }
 

javascript/ql/test/query-tests/Security/CWE-1004/CookieWithoutHttpOnly.expected

@@ -8,6 +8,7 @@
 | test_httpserver.js:7:37:7:48 | "auth=ninja" | Cookie attribute 'HttpOnly' is not set to true. |
 | test_httpserver.js:27:37:27:70 | ["auth= ... cript"] | Cookie attribute 'HttpOnly' is not set to true. |
 | test_httpserver.js:57:37:57:80 | ["auth= ... cript"] | Cookie attribute 'HttpOnly' is not set to true. |
+| test_httpserver.js:87:37:87:59 | `sessio ... {attr}` | Cookie attribute 'HttpOnly' is not set to true. |
 | test_responseCookie.js:15:5:20:10 | res.coo ...      }) | Cookie attribute 'HttpOnly' is not set to true. |
 | test_responseCookie.js:25:5:28:10 | res.coo ...      }) | Cookie attribute 'HttpOnly' is not set to true. |
 | test_responseCookie.js:48:5:48:43 | res.coo ... ptions) | Cookie attribute 'HttpOnly' is not set to true. |

javascript/ql/test/query-tests/Security/CWE-1004/test_httpserver.js

@@ -68,4 +68,24 @@ function test7() {
         res.writeHead(200, { 'Content-Type': 'text/plain' });
         res.end('ok');
     });
+}
+
+function test8() {
+    const server = http.createServer((req, res) => {
+        res.setHeader('Content-Type', 'text/html');
+        let attr = "; httponly"
+        res.setHeader("Set-Cookie", `session=ninja ${attr}`); // Good, httponly string expression
+        res.writeHead(200, { 'Content-Type': 'text/plain' });
+        res.end('ok');
+    });
+}
+
+function test9() {
+    const server = http.createServer((req, res) => {
+        res.setHeader('Content-Type', 'text/html');
+        let attr = "; secure"
+        res.setHeader("Set-Cookie", `session=ninja ${attr}`); // Bad, not httponly string expression
+        res.writeHead(200, { 'Content-Type': 'text/plain' });
+        res.end('ok');
+    });
 }