Add Codeql query to detect if cookies are sent without the flag being set

javascript/ql/src/experimental/Security/CWE-614/InsecureCookie.qhelp

@@ -0,0 +1,84 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Failing to set the 'secure' flag on a cookie can cause it to be sent in cleartext.
+This makes it easier for an attacker to intercept.</p>
+</overview>
+<recommendation>
+
+<p>Always set the <code>secure</code> flag to `true` on a cookie before adding it
+to an HTTP response (if the default value is `false`).</p>
+
+</recommendation>
+<example>
+
+<p>In the first example the `secure` flag is set to `false` using the express middleware `cookie-session`. 
+In the second example the `secure` flag is set to `true` (it is set `false` by default for HTTP, `true` by default for HTTPS).</p>
+
+<sample src="examples/cookie-session_bad.js" />
+<sample src="examples/cookie-session_good.js" />
+
+</example>
+
+<example>
+
+<p>The first four examples show four ways of adding a cookie using the express middleware `express-session`. 
+Since the default value for the flag `secure` is false, each example shows a possible scenario where a cookie is set with
+the `secure` to `false`. 
+In the last example the `secure` flag is set to `true`.</p>
+
+<sample src="examples/express-session_bad1_false.js" />
+<sample src="examples/express-session_bad2_notSet.js" />
+<sample src="examples/express-session_bad3_setEmpty.js" />
+<sample src="examples/express-session_bad4.js" />
+<sample src="examples/express-session_good.js" />
+
+</example>
+
+<example>
+
+<p>The first two examples show two ways of adding a cookie using the method `response.cookie`. 
+In both cases the `secure` flag is to `false`. 
+In the last example the `secure` flag is set to `true`.</p>
+
+<sample src="examples/express_response-cookie_bad1.js" />
+<sample src="examples/express_response-cookie_bad2.js" />
+<sample src="examples/express_response-cookie_good1.js" />
+
+</example>
+
+
+<example>
+
+<p>The first example shows when the `secure` flag is set using the method `Set-Cookie` header of an `HTTP` response. 
+In this case the `secure` flag is not set. 
+In the last example the `secure` flag is set.</p>
+
+<sample src="examples/httpserver_bad.js" />
+<sample src="examples/httpserver_good.js" />
+
+</example>
+
+<example>
+
+<p>In the first example the `secure` flag is set to `false` using the `js-cookie` library. 
+In the second example the `secure` flag is set to `true`.</p>
+
+<sample src="examples/jsCookie_bad.js" />
+<sample src="examples/jsCookie_good.js" />
+
+</example>
+
+<references>
+
+<li>Production Best Practices: Security:<a href="https://expressjs.com/en/advanced/best-practice-security.html#use-cookies-securely">Use cookies securely</a>.</li>
+<li>NodeJS security cheat sheet:<a href="https://cheatsheetseries.owasp.org/cheatsheets/Nodejs_Security_Cheat_Sheet.html#set-cookie-flags-appropriately">Set cookie flags appropriately</a>.</li>
+<li>express-session:<a href="https://github.com/expressjs/session#cookiesecure">cookie.secure</a>.</li>
+<li>cookie-session:<a href="https://github.com/expressjs/cookie-session#cookie-options">Cookie Options</a>.</li>
+<li><a href="https://expressjs.com/en/api.html#res.cookie">express response.cookie</a>.</li>
+<li><a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie">Set-Cookie</a>.</li>
+<li><a href="https://github.com/js-cookie/js-cookie">js-cookie</a>.</li>
+</references>
+</qhelp>

javascript/ql/src/experimental/Security/CWE-614/InsecureCookie.ql

@@ -0,0 +1,19 @@
+/**
+ * @name Failure to set secure cookies
+ * @description Insecure cookies may be sent in cleartext, which makes them vulnerable to
+ *              interception.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @id js/insecure-cookie
+ * @tags security
+ *       external/cwe/cwe-614
+ */
+
+import javascript
+import InsecureCookie::InsecureCookie
+
+from InsecureCookies insecureCookies
+where insecureCookies.isInsecure()
+select "Cookie is added to response without the 'secure' flag being set to true (using " +
+    insecureCookies.getKind() + ").", insecureCookies

javascript/ql/src/experimental/Security/CWE-614/InsecureCookie.qll

@@ -0,0 +1,161 @@
+/**
+ * Provides classes for reasoning about cookies added to response without the 'secure' flag being set.
+ * A cookie without the 'secure' flag being set can be intercepted and read by a malicious user.
+ */
+
+import javascript
+
+module InsecureCookie {
+  /**
+   * `secure` property of the cookie options.
+   */
+  string flag() { result = "secure" }
+
+  /**
+   * Abstract class to represent different cases of insecure cookie settings.
+   */
+  abstract class InsecureCookies extends DataFlow::Node {
+    /**
+     * The name of the middleware/library used to set the cookie.
+     */
+    abstract string getKind();
+
+    /**
+     * The `cookie` options.
+     */
+    abstract DataFlow::Node getCookieOptionsArgument();
+
+    /**
+     * Predicate that determines if a cookie is insecure.
+     */
+    abstract predicate isInsecure();
+  }
+
+  /**
+   * A cookie set using the `express` module `cookie-session` (https://github.com/expressjs/cookie-session).
+   * The flag `secure` is set to `false` by default for HTTP, `true` by default for HTTPS (https://github.com/expressjs/cookie-session#cookie-options).
+   */
+  class InsecureCookieSession extends ExpressLibraries::CookieSession::MiddlewareInstance,
+    InsecureCookies {
+    InsecureCookieSession() { this instanceof ExpressLibraries::CookieSession::MiddlewareInstance }
+
+    override string getKind() { result = "cookie-session" }
+
+    override DataFlow::SourceNode getCookieOptionsArgument() {
+      result = this.getOption("cookie").(DataFlow::SourceNode)
+    }
+
+    DataFlow::Node getCookieFlagValue(string flag) {
+      result = this.getCookieOptionsArgument().getAPropertyWrite(flag).getRhs()
+    }
+
+    // A cookie is insecure if the `secure` flag is explicitly set to `false`.
+    override predicate isInsecure() { getCookieFlagValue(flag()).mayHaveBooleanValue(false) }
+  }
+
+  /**
+   * A cookie set using the `express` module `express-session` (https://github.com/expressjs/session).
+   * The flag `secure` is not set by default (https://github.com/expressjs/session#cookiesecure).
+   * The default value for cookie options is { path: '/', httpOnly: true, secure: false, maxAge: null }.
+   */
+  class InsecureExpressSessionCookie extends ExpressLibraries::ExpressSession::MiddlewareInstance,
+    InsecureCookies {
+    override string getKind() { result = "express-session" }
+
+    override DataFlow::SourceNode getCookieOptionsArgument() {
+      result = this.getOption("cookie").(DataFlow::SourceNode)
+    }
+
+    DataFlow::Node getCookieFlagValue(string flag) {
+      result = this.getCookieOptionsArgument().getAPropertyWrite(flag).getRhs()
+    }
+
+    // A cookie is insecure if there are not cookie options with the `secure` flag set to `true`.
+    override predicate isInsecure() {
+      not exists(DataFlow::SourceNode cookieOptions |
+        cookieOptions = this.getCookieOptionsArgument() and
+        getCookieFlagValue(flag()).mayHaveBooleanValue(true)
+      ) and
+      not getCookieFlagValue(flag()).mayHaveStringValue("auto")
+    }
+  }
+
+  /**
+   * A cookie set using `response.cookie` from `express` module (https://expressjs.com/en/api.html#res.cookie).
+   */
+  class InsecureExpressCookieResponse extends InsecureCookies {
+    InsecureExpressCookieResponse() {
+      this = any(Express::ResponseExpr response).flow().getALocalSource().getAMemberCall("cookie")
+    }
+
+    override string getKind() { result = "response.cookie" }
+
+    override DataFlow::SourceNode getCookieOptionsArgument() {
+      result = this.(DataFlow::InvokeNode).getLastArgument().getALocalSource()
+    }
+
+    DataFlow::Node getCookieFlagValue(string flag) {
+      result = this.getCookieOptionsArgument().getAPropertyWrite(flag).getRhs()
+    }
+
+    // A cookie is insecure if there are not cookie options with the `secure` flag set to `true`.
+    override predicate isInsecure() {
+      not exists(DataFlow::SourceNode cookieOptions |
+        cookieOptions = this.getCookieOptionsArgument() and
+        getCookieFlagValue(flag()).mayHaveBooleanValue(true)
+      )
+    }
+  }
+
+  /**
+   * A cookie set using `Set-Cookie` header of an `HTTP` response (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie).
+   */
+  class InsecureSetCookieHeader extends InsecureCookies {
+    InsecureSetCookieHeader() {
+      this.asExpr() = any(HTTP::SetCookieHeader setCookie).getHeaderArgument()
+    }
+
+    override string getKind() { result = "set-cookie header" }
+
+    override DataFlow::Node getCookieOptionsArgument() {
+      result.asExpr() = this.asExpr().(ArrayExpr).getAnElement()
+    }
+
+    // A cookie is insecure if the 'secure' flag is not specified in the cookie definition.
+    override predicate isInsecure() {
+      not exists(string s |
+        getCookieOptionsArgument().mayHaveStringValue(s) and
+        s.matches("%; secure%")
+      )
+    }
+  }
+
+  /**
+   * A cookie set using `js-cookie` library (https://github.com/js-cookie/js-cookie).
+   */
+  class InsecureJsCookie extends InsecureCookies {
+    InsecureJsCookie() {
+      this = DataFlow::globalVarRef("Cookie").getAMemberCall("set") or
+      this = DataFlow::globalVarRef("Cookie").getAMemberCall("noConflict").getAMemberCall("set") or
+      this = DataFlow::moduleMember("js-cookie", "set").getACall()
+    }
+
+    override string getKind() { result = "js-cookie" }
+
+    override DataFlow::SourceNode getCookieOptionsArgument() {
+      result = this.(DataFlow::CallNode).getArgument(2).getALocalSource()
+    }
+
+    DataFlow::Node getCookieFlagValue(string flag) {
+      result = this.getCookieOptionsArgument().getAPropertyWrite(flag).getRhs()
+    }
+
+    // A cookie is insecure if there are not cookie options with the `secure` flag set to `true`.
+    override predicate isInsecure() {
+      not exists(DataFlow::SourceNode cookieOptions |
+        cookieOptions = this.getCookieOptionsArgument() and
+        getCookieFlagValue(flag()).mayHaveBooleanValue(true)
+      )
+    }
+  }
+}

javascript/ql/src/experimental/Security/CWE-614/examples/cookie-session_bad.js

@@ -0,0 +1,17 @@
+const session = require('cookie-session')
+const express = require('express')
+const app = express()
+
+const expiryDate = new Date(Date.now() + 60 * 60 * 1000)
+
+app.use(session({
+    name: 'session',
+    keys: ['key1', 'key2'],
+    cookie: {
+        secure: false, // BAD
+        httpOnly: true,
+        domain: 'example.com',
+        path: 'foo/bar',
+        expires: expiryDate
+    }
+}))

javascript/ql/src/experimental/Security/CWE-614/examples/cookie-session_good.js

@@ -0,0 +1,17 @@
+const session = require('cookie-session')
+const express = require('express')
+const app = express()
+
+const expiryDate = new Date(Date.now() + 60 * 60 * 1000)
+
+app.use(session({
+    name: 'session',
+    keys: ['key1', 'key2'],
+    cookie: {
+        secure: true, // GOOD: false by default for HTTP, true by default for HTTPS
+        httpOnly: true,
+        domain: 'example.com',
+        path: 'foo/bar',
+        expires: expiryDate
+    }
+}))

javascript/ql/src/experimental/Security/CWE-614/examples/express-session_bad1_false.js

@@ -0,0 +1,7 @@
+const app = express()
+const session = require('express-session')
+
+app.use(session({
+    secret: 'secret',
+    cookie: { secure: false } // BAD
+}))

javascript/ql/src/experimental/Security/CWE-614/examples/express-session_bad2_notSet.js

@@ -0,0 +1,7 @@
+const app = express()
+const session = require('express-session')
+
+app.use(session({
+    secret: 'secret'
+    // BAD: in this case the default value of `secure` flag is `false`
+}))

javascript/ql/src/experimental/Security/CWE-614/examples/express-session_bad3_setEmpty.js

@@ -0,0 +1,7 @@
+const app = express()
+const session = require('express-session')
+
+app.use(session({
+    secret: 'secret',
+    cookie: {} // BAD: in this case the default value of `secure` flag is `false`
+}))

javascript/ql/src/experimental/Security/CWE-614/examples/express-session_bad4.js

@@ -0,0 +1,9 @@
+const app = express()
+const session = require('express-session')
+
+const sess = {
+    secret: 'secret',
+    cookie: { secure: false } // BAD
+}
+
+app.use(session(sess))

javascript/ql/src/experimental/Security/CWE-614/examples/express-session_good.js

@@ -0,0 +1,9 @@
+const app = express()
+const session = require('express-session')
+
+app.set('trust proxy', 1)
+
+app.use(session({
+    secret: 'secret',
+    cookie: { secure: true } // GOOD
+}))

javascript/ql/src/experimental/Security/CWE-614/examples/express_response-cookie_bad1.js

@@ -0,0 +1,12 @@
+const express = require('express')
+const app = express()
+
+app.get('/', function (req, res, next) {
+    res.cookie('name', 'value',
+        {
+            maxAge: 9000000000,
+            httpOnly: true,
+            secure: false // BAD
+        });
+    res.end('ok')
+})

javascript/ql/src/experimental/Security/CWE-614/examples/express_response-cookie_bad2.js

@@ -0,0 +1,12 @@
+const express = require('express')
+const app = express()
+
+app.get('/', function (req, res, next) {
+    let options = {
+        maxAge: 9000000000,
+        httpOnly: true,
+        secure: false // BAD
+    }
+    res.cookie('name', 'value', options);
+    res.end('ok')
+})

javascript/ql/src/experimental/Security/CWE-614/examples/express_response-cookie_good1.js

@@ -0,0 +1,12 @@
+const express = require('express')
+const app = express()
+
+app.get('/', function (req, res, next) {
+    res.cookie('name', 'value',
+        {
+            maxAge: 9000000000,
+            httpOnly: true,
+            secure: true // GOOD
+        });
+    res.end('ok')
+})

javascript/ql/src/experimental/Security/CWE-614/examples/httpserver_bad.js

@@ -0,0 +1,8 @@
+const http = require('http');
+const server = http.createServer((req, res) => {
+    res.setHeader('Content-Type', 'text/html');
+    // BAD
+    res.setHeader("Set-Cookie", ["type=ninja", "language=javascript"]);
+    res.writeHead(200, { 'Content-Type': 'text/plain' });
+    res.end('ok');
+});

javascript/ql/src/experimental/Security/CWE-614/examples/httpserver_good.js

@@ -0,0 +1,8 @@
+const http = require('http');
+const server = http.createServer((req, res) => {
+    res.setHeader('Content-Type', 'text/html');
+    // GOOD
+    res.setHeader("Set-Cookie", ["type=ninja; Secure", "language=javascript; secure"]);
+    res.writeHead(200, { 'Content-Type': 'text/plain' });
+    res.end('ok');
+});

javascript/ql/src/experimental/Security/CWE-614/examples/jsCookie_bad.js

@@ -0,0 +1,2 @@
+const js_cookie = require('js-cookie')
+js_cookie.set('key', 'value', { secure: false }); // BAD

javascript/ql/src/experimental/Security/CWE-614/examples/jsCookie_good.js

@@ -0,0 +1,2 @@
+const js_cookie = require('js-cookie')
+js_cookie.set('key', 'value', { secure: true });