documentation overhaul on client-exposed-cookie (and restricting it to server-side)

Author: erik-krogh

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

@@ -24,9 +24,25 @@ module CookieWrites {
 
     /**
      * Holds if the cookie is likely an authentication cookie or otherwise sensitive.
-     * Can never hold for client-side cookies. TODO: Or can it...?
+     * Can never hold for client-side cookies.
      */
     abstract predicate isSensitive();
+
+    /**
+     * Holds if the cookie write happens on a server, that is `httpOnly` flag is relevant.
+     */
+    predicate isServerSide() {
+      any() // holds by default. Client-side cookie writes should extend ClientSideCookieWrite.
+    }
+  }
+
+  /**
+   * A client-side write to a cookie.
+   */
+  abstract class ClientSideCookieWrite extends CookieWrite {
+    final override predicate isHttpOnly() { none() }
+
+    final override predicate isServerSide() { none() }
   }
 
   /**
@@ -43,6 +59,7 @@ module CookieWrites {
 /**
  * A model of the `js-cookie` library (https://github.com/js-cookie/js-cookie).
  */
+// TODO: Writes to document.cookie.
 private module JsCookie {
   /**
    * Gets a function call that invokes method `name` of the `js-cookie` library.
@@ -61,7 +78,8 @@ private module JsCookie {
     }
   }
 
-  class WriteAccess extends PersistentWriteAccess, DataFlow::CallNode, CookieWrites::CookieWrite {
+  class WriteAccess extends PersistentWriteAccess, DataFlow::CallNode,
+    CookieWrites::ClientSideCookieWrite {
     WriteAccess() { this = libMemberCall("set") }
 
     string getKey() { getArgument(0).mayHaveStringValue(result) }
@@ -78,8 +96,6 @@ private module JsCookie {
           this.getArgument(0).mayHaveStringValue(s)
         ), _)
     }
-
-    override predicate isHttpOnly() { none() } // js-cookie is browser side library and doesn't support HttpOnly
   }
 }
 

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

@@ -0,0 +1,42 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Authentication cookies stored by a server can be accessed by a client if the <code>httpOnly</code> flag is not set.
+<p>
+<p>
+An attacker that manages a cross-site scripting (XSS) attack can read the cookie and hijack the session.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Set the <code>httpOnly</code> flag on all cookies that are not needed by the client.
+</p>
+</recommendation>
+
+<references>
+
+<example>
+<p>
+The following example stores an authentication token in a cookie that can 
+viewed by the client.
+</p>
+<sample src="examples/ClientExposedCookieGood.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"/>
+</example>
+
+<references>
+<li>ExpressJS: <a href="https://expressjs.com/en/advanced/best-practice-security.html#use-cookies-securely">Use cookies securely</a>.</li>
+<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Nodejs_Security_Cheat_Sheet.html#set-cookie-flags-appropriately">Set cookie flags appropriately</a>.</li>
+<li><a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie">Set-Cookie</a>.</li>
+</references>
+
+</qhelp>

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

@@ -0,0 +1,20 @@
+/**
+ * @name Sensitive server cookie exposed to the client
+ * @description Sensitive cookies set by a server can be read by the client if the `httpOnly` flag is not set.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 5.0
+ * @precision high
+ * @id js/client-exposed-cookie
+ * @tags security
+ *       external/cwe/cwe-1004
+ */
+
+import javascript
+
+from CookieWrites::CookieWrite cookie
+where
+  cookie.isSensitive() and
+  cookie.isServerSide() and
+  not cookie.isHttpOnly()
+select cookie, "Sensitive server cookie is missing 'httpOnly' flag."

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

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

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

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

javascript/ql/src/Security/CWE-1004/examples/ClientExposedCookieBad.js

@@ -0,0 +1,19 @@
+| tst-httpOnly.js:11:9:15:2 | session ...  BAD\\n}) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:29:9:29:21 | session(sess) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:38:9:38:22 | session(sess2) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:47:9:47:22 | session(sess3) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:51:9:55:2 | session ...  BAD\\n}) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:68:5:73:10 | res.coo ...      }) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:78:5:81:10 | res.coo ...      }) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:101:5:101:43 | res.coo ... ptions) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:109:5:109:43 | res.coo ... ptions) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:118:5:118:43 | res.coo ... ptions) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:137:5:137:43 | res.coo ... ptions) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:148:5:148:41 | res.coo ... ptions) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:159:5:159:43 | res.coo ... ptions) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:170:5:170:40 | res.coo ... ptions) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:209:37:209:51 | "authKey=ninja" | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:229:38:229:52 | "authKey=ninja" | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:289:37:289:59 | `authKe ... {attr}` | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:303:9:307:2 | session ...  BAD\\n}) | Sensitive server cookie is missing 'httpOnly' flag. |
+| tst-httpOnly.js:320:9:324:2 | session ... tter\\n}) | Sensitive server cookie is missing 'httpOnly' flag. |

javascript/ql/src/Security/CWE-1004/examples/ClientExposedCookieGood.js

@@ -0,0 +1 @@
+Security/CWE-1004/ClientExposedCookie.ql