Merge pull request #7721 from erik-krogh/CWE-1275

JS: add a js/samesite-none-cookie cookie

Author: erik-krogh

javascript/ql/lib/semmle/javascript/frameworks/CookieLibraries.qll

@@ -27,6 +27,12 @@ module CookieWrites {
      */
     abstract predicate isSensitive();
 
+    /**
+     * Gets the SameSite attribute of the cookie if present.
+     * Either "Strict", "Lax" or "None".
+     */
+    abstract string getSameSite();
+
     /**
      * Holds if the cookie write happens on a server, i.e. the `httpOnly` flag is relevant.
      */
@@ -95,6 +101,31 @@ private predicate hasCookieAttribute(string s, string attribute) {
   s.regexpMatch("(?i).*;\\s*" + attribute + "\\b\\s*;?.*$")
 }
 
+/**
+ * Gets the value for a `Set-Cookie` header attribute.
+ */
+bindingset[s, attribute]
+private string getCookieValue(string s, string attribute) {
+  result = s.regexpCapture("(?i).*;\\s*" + attribute + "=(\\w+)\\b\\s*;?.*$", 1)
+}
+
+/**
+ * Gets the "SameSite" value for a given `node`.
+ * Converts boolean values to the corresponding string value.
+ *
+ * Not all libraries support boolean values for the `SameSite` attribute,
+ * but here we assume that they do.
+ */
+private string getSameSiteValue(DataFlow::Node node) {
+  node.mayHaveStringValue(result)
+  or
+  node.mayHaveBooleanValue(true) and
+  result = "Strict"
+  or
+  node.mayHaveBooleanValue(false) and
+  result = "Lax"
+}
+
 /**
  * A model of the `js-cookie` library (https://github.com/js-cookie/js-cookie).
  */
@@ -105,7 +136,9 @@ private module JsCookie {
   DataFlow::CallNode libMemberCall(string name) {
     result = DataFlow::globalVarRef("Cookie").getAMemberCall(name) or
     result = DataFlow::globalVarRef("Cookie").getAMemberCall("noConflict").getAMemberCall(name) or
-    result = DataFlow::moduleMember("js-cookie", name).getACall()
+    result = DataFlow::moduleMember("js-cookie", name).getACall() or
+    // es-cookie behaves basically the same as js-cookie
+    result = DataFlow::moduleMember("es-cookie", name).getACall()
   }
 
   class ReadAccess extends PersistentReadAccess, DataFlow::CallNode {
@@ -132,6 +165,10 @@ private module JsCookie {
     }
 
     override predicate isSensitive() { canHaveSensitiveCookie(this.getArgument(0)) }
+
+    override string getSameSite() {
+      result = getSameSiteValue(this.getOptionArgument(2, "sameSite"))
+    }
   }
 }
 
@@ -173,6 +210,15 @@ private module BrowserCookies {
     }
 
     override predicate isSensitive() { canHaveSensitiveCookie(this.getArgument(0)) }
+
+    override string getSameSite() {
+      result = getSameSiteValue(this.getOptionArgument(2, "samesite"))
+      or
+      // or, an explicit default has been set
+      DataFlow::moduleMember("browser-cookies", "defaults")
+          .getAPropertyWrite("samesite")
+          .mayHaveStringValue(result)
+    }
   }
 }
 
@@ -211,6 +257,10 @@ private module LibCookie {
     }
 
     override predicate isSensitive() { canHaveSensitiveCookie(this.getArgument(0)) }
+
+    override string getSameSite() {
+      result = getSameSiteValue(this.getOptionArgument(2, "sameSite"))
+    }
   }
 }
 
@@ -242,6 +292,10 @@ private module ExpressCookies {
         not value.mayHaveBooleanValue(false) // anything but `false` is accepted as being maybe true
       )
     }
+
+    override string getSameSite() {
+      result = getSameSiteValue(this.getOptionArgument(2, "sameSite"))
+    }
   }
 
   /**
@@ -268,6 +322,8 @@ private module ExpressCookies {
       // A cookie is httpOnly if the `httpOnly` flag is not explicitly set to `false`.
       not this.getCookieFlagValue(CookieWrites::httpOnly()).mayHaveBooleanValue(false)
     }
+
+    override string getSameSite() { result = getSameSiteValue(this.getCookieFlagValue("sameSite")) }
   }
 
   /**
@@ -297,6 +353,8 @@ private module ExpressCookies {
       // A cookie is httpOnly if the `httpOnly` flag is not explicitly set to `false`.
       not this.getCookieFlagValue(CookieWrites::httpOnly()).mayHaveBooleanValue(false)
     }
+
+    override string getSameSite() { result = getSameSiteValue(this.getCookieFlagValue("sameSite")) }
   }
 }
 
@@ -339,6 +397,8 @@ private class HTTPCookieWrite extends CookieWrites::CookieWrite {
   }
 
   override predicate isSensitive() { canHaveSensitiveCookie(this) }
+
+  override string getSameSite() { result = getCookieValue(header, "SameSite") }
 }
 
 /**
@@ -365,4 +425,6 @@ private class DocumentCookieWrite extends CookieWrites::ClientSideCookieWrite {
   }
 
   override predicate isSensitive() { canHaveSensitiveCookie(write.getRhs()) }
+
+  override string getSameSite() { result = getCookieValue(cookie, "SameSite") }
 }

javascript/ql/src/Security/CWE-1004/ClientExposedCookie.qhelp

@@ -23,12 +23,12 @@ Set the <code>httpOnly</code> flag on all cookies that are not needed by the cli
 The following example stores an authentication token in a cookie that can 
 be viewed by the client.
 </p>
-<sample src="examples/ClientExposedCookieGood.js"/>
+<sample src="examples/ClientExposedCookieBad.js"/>
 <p>
 To force the cookie to be transmitted using SSL, set the <code>secure</code>
 attribute on the cookie.
 </p>
-<sample src="examples/ClientExposedCookieBad.js"/>
+<sample src="examples/ClientExposedCookieGood.js"/>
 </example>
 
 <references>

javascript/ql/src/Security/CWE-1275/SameSiteNoneCookie.qhelp

@@ -0,0 +1,43 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Authentication cookies where the SameSite attribute is set to "None" can 
+potentially be used to perform Cross-Site Request Forgery (CSRF) attacks 
+if no other CSRF protections are in place.
+</p>
+<p>
+With SameSite set to "None", a third party website may create an authorized cross-site request
+that includes the cookie.
+Such a cross-site request can allow that website to perform actions on behalf of a user.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Set the <code>SameSite</code> attribute to <code>Strict</code> on all sensitive cookies.
+</p>
+</recommendation>
+
+<example>
+<p>
+The following example stores an authentication token in a cookie where the <code>SameSite</code>
+attribute is set to <code>None</code>.
+</p>
+<sample src="examples/SameSiteCookieBad.js"/>
+<p>
+To prevent the cookie from being included in cross-site requests, set the <code>SameSite</code>
+attribute to <code>Strict</code>.
+</p>
+<sample src="examples/SameSiteCookieGood.js"/>
+</example>
+
+<references>
+<li>MDN Web Docs: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite">SameSite cookies</a>.</li>
+<li>OWASP: <a href="https://owasp.org/www-community/SameSite">SameSite</a>.</li>
+</references>
+
+</qhelp>

javascript/ql/src/Security/CWE-1275/SameSiteNoneCookie.ql

@@ -0,0 +1,21 @@
+/**
+ * @name Sensitive cookie without SameSite restrictions
+ * @description Sensitive cookies where the SameSite attribute is set to "None" can
+ *              in some cases allow for Cross-Site Request Forgery (CSRF) attacks.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 5.0
+ * @precision medium
+ * @id js/samesite-none-cookie
+ * @tags security
+ *       external/cwe/cwe-1275
+ */
+
+import javascript
+
+from CookieWrites::CookieWrite cookie
+where
+  cookie.isSensitive() and
+  cookie.isSecure() and // `js/clear-text-cookie` will report it if the cookie is not secure.
+  cookie.getSameSite().toLowerCase() = "none"
+select cookie, "Sensitive cookie with SameSite set to 'None'"

javascript/ql/src/Security/CWE-1275/examples/SameSiteCookieBad.js

@@ -0,0 +1,7 @@
+const http = require('http');
+
+const server = http.createServer((req, res) => {
+    res.setHeader("Set-Cookie", `authKey=${makeAuthkey()}; secure; httpOnly; SameSite=None`);
+    res.writeHead(200, { 'Content-Type': 'text/html' });
+    res.end('<h2>Hello world</h2>');
+});

javascript/ql/src/Security/CWE-1275/examples/SameSiteCookieGood.js

@@ -0,0 +1,7 @@
+const http = require('http');
+
+const server = http.createServer((req, res) => {
+    res.setHeader("Set-Cookie", `authKey=${makeAuthkey()}; secure; httpOnly; SameSite=Strict`);
+    res.writeHead(200, { 'Content-Type': 'text/html' });
+    res.end('<h2>Hello world</h2>');
+});

javascript/ql/src/change-notes/2022-01-24-samesite-cookie.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* A new query `js/samesite-none-cookie` has been added. The query detects when the SameSite attribute is set to None on a sensitive cookie.

javascript/ql/test/query-tests/Security/CWE-1275/SameSiteNoneCookie.expected

@@ -0,0 +1,8 @@
+| tst-sameSite.js:4:3:8:4 | Cookies ... OK\\n  }) | Sensitive cookie with SameSite set to 'None' |
+| tst-sameSite.js:20:3:25:4 | cookies ... OK\\n  }) | Sensitive cookie with SameSite set to 'None' |
+| tst-sameSite.js:38:19:43:4 | cookie. ... ",\\n  }) | Sensitive cookie with SameSite set to 'None' |
+| tst-sameSite.js:58:3:63:4 | res.coo ... OK\\n  }) | Sensitive cookie with SameSite set to 'None' |
+| tst-sameSite.js:76:3:82:4 | session ... OK\\n  }) | Sensitive cookie with SameSite set to 'None' |
+| tst-sameSite.js:98:3:106:4 | express ... },\\n  }) | Sensitive cookie with SameSite set to 'None' |
+| tst-sameSite.js:126:33:126:70 | "authKe ... Secure" | Sensitive cookie with SameSite set to 'None' |
+| tst-sameSite.js:134:3:134:17 | document.cookie | Sensitive cookie with SameSite set to 'None' |

javascript/ql/test/query-tests/Security/CWE-1275/SameSiteNoneCookie.qlref

@@ -0,0 +1 @@
+Security/CWE-1275/SameSiteNoneCookie.ql

javascript/ql/test/query-tests/Security/CWE-1275/tst-sameSite.js

@@ -0,0 +1,137 @@
+import * as Cookies from "es-cookie";
+
+function esCookies() {
+  Cookies.set("authkey", "value", {
+    secure: true,
+    httpOnly: true,
+    sameSite: "None", // NOT OK
+  });
+
+  Cookies.set("authkey", "value", {
+    secure: true,
+    httpOnly: true,
+    sameSite: "Strict", // OK
+  });
+}
+
+function browserCookies() {
+  var cookies = require("browser-cookies");
+
+  cookies.set("authkey", "value", {
+    expires: 365,
+    secure: true,
+    httponly: true,
+    samesite: "None", // NOT OK
+  });
+
+  cookies.set("authkey", "value", {
+    expires: 365,
+    secure: true,
+    httponly: true,
+    samesite: "Strict", // OK
+  });
+}
+
+function cookie() {
+  var cookie = require("cookie");
+
+  var setCookie = cookie.serialize("authkey", "value", {
+    maxAge: 9000000000,
+    httpOnly: true,
+    secure: true,
+    sameSite: "None",
+  });
+
+  var setCookie = cookie.serialize("authkey", "value", {
+    maxAge: 9000000000,
+    httpOnly: true,
+    secure: true,
+    sameSite: true, // OK
+  });
+}
+
+const express = require("express");
+const app = express();
+const session = require("cookie-session");
+
+app.get("/a", function (req, res, next) {
+  res.cookie("authkey", "value", {
+    maxAge: 9000000000,
+    httpOnly: true,
+    secure: true,
+    sameSite: "None", // NOT OK
+  });
+
+  res.cookie("session", "value", {
+    maxAge: 9000000000,
+    httpOnly: true,
+    secure: true,
+    sameSite: "Strict", // OK
+  });
+
+  res.end("ok");
+});
+
+app.use(
+  session({
+    name: "session",
+    keys: ["key1", "key2"],
+    httpOnly: true,
+    secure: true,
+    sameSite: "None", // NOT OK
+  })
+);
+
+app.use(
+  session({
+    name: "session",
+    keys: ["key1", "key2"],
+    httpOnly: true,
+    secure: true,
+    sameSite: "Strict", // OK
+  })
+);
+
+const expressSession = require("express-session");
+
+app.use(
+  expressSession({
+    name: "session",
+    keys: ["key1", "key2"],
+    cookie: {
+      httpOnly: true,
+      secure: true,
+      sameSite: "None", // NOT OK
+    },
+  })
+);
+
+app.use(
+  expressSession({
+    name: "session",
+    keys: ["key1", "key2"],
+    cookie: {
+      httpOnly: true,
+      secure: true,
+      sameSite: "Strict", // OK
+    },
+  })
+);
+
+const http = require("http");
+
+function test1() {
+  const server = http.createServer((req, res) => {
+    res.setHeader("Content-Type", "text/html");
+    res.setHeader("Set-Cookie", "authKey=ninja; SameSite=None; Secure"); // NOT OK
+    res.setHeader("Set-Cookie", "authKey=ninja; SameSite=Strict; Secure"); // OK
+    res.writeHead(200, { "Content-Type": "text/plain" });
+    res.end("ok");
+  });
+}
+
+function documentCookie() {
+  document.cookie = "authKey=ninja; SameSite=None; Secure"; // NOT OK
+  document.cookie = "authKey=ninja; SameSite=Strict; Secure"; // OK
+}
+