only report clear-text cookies for sensitive cookies

Author: erik-krogh

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

@@ -73,7 +73,11 @@ private module JsCookie {
       this.getOptionArgument(2, CookieWrites::secure()).mayHaveBooleanValue(true)
     }
 
-    override predicate isSensitive() { none() } // TODO: Maybe it can be sensitive?
+    override predicate isSensitive() {
+      HeuristicNames::nameIndicatesSensitiveData(any(string s |
+          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-614/InsecureCookie.ql

@@ -12,7 +12,10 @@
 
 import javascript
 
+// TODO: Rename to ClearTextCookie?
 from DataFlow::Node node
-// TODO: Only for sensitive cookies? (e.g. auth cookies)
-where exists(CookieWrites::CookieWrite cookie | cookie = node | not cookie.isSecure())
+where
+  exists(CookieWrites::CookieWrite cookie | cookie = node |
+    cookie.isSensitive() and not cookie.isSecure()
+  )
 select node, "Cookie is added to response without the 'secure' flag being set to true"

javascript/ql/test/query-tests/Security/CWE-614/InsecureCookies.expected

@@ -1,10 +1,9 @@
 | tst-cleartextCookie.js:5:5:10:10 | res.coo ...      }) | Cookie is added to response without the 'secure' flag being set to true |
-| tst-cleartextCookie.js:20:5:20:40 | res.coo ... ptions) | Cookie is added to response without the 'secure' flag being set to true |
-| tst-cleartextCookie.js:35:1:35:48 | js_cook ... alse }) | Cookie is added to response without the 'secure' flag being set to true |
-| tst-cleartextCookie.js:44:37:44:48 | "type=ninja" | Cookie is added to response without the 'secure' flag being set to true |
-| tst-cleartextCookie.js:64:38:64:49 | "type=ninja" | Cookie is added to response without the 'secure' flag being set to true |
-| tst-cleartextCookie.js:64:52:64:72 | "langua ... script" | Cookie is added to response without the 'secure' flag being set to true |
-| tst-cleartextCookie.js:94:60:94:80 | "langua ... script" | Cookie is added to response without the 'secure' flag being set to true |
+| tst-cleartextCookie.js:20:5:20:43 | res.coo ... ptions) | Cookie is added to response without the 'secure' flag being set to true |
+| tst-cleartextCookie.js:35:1:35:52 | js_cook ... alse }) | Cookie is added to response without the 'secure' flag being set to true |
+| tst-cleartextCookie.js:44:37:44:51 | "authKey=ninja" | Cookie is added to response without the 'secure' flag being set to true |
+| tst-cleartextCookie.js:64:38:64:52 | "authKey=ninja" | Cookie is added to response without the 'secure' flag being set to true |
+| tst-cleartextCookie.js:94:60:94:72 | "authKey=foo" | Cookie is added to response without the 'secure' flag being set to true |
 | tst-cleartextCookie.js:104:9:107:2 | session ... T OK\\n}) | Cookie is added to response without the 'secure' flag being set to true |
 | tst-cleartextCookie.js:109:9:112:2 | session ... T OK\\n}) | Cookie is added to response without the 'secure' flag being set to true |
 | tst-cleartextCookie.js:114:9:117:2 | session ... T OK\\n}) | Cookie is added to response without the 'secure' flag being set to true |

javascript/ql/test/query-tests/Security/CWE-614/tst-cleartextCookie.js

@@ -2,7 +2,7 @@ const express = require('express')
 const app = express()
 
 app.get('/a', function (req, res, next) {
-    res.cookie('name', 'value',
+    res.cookie('authkey', 'value',
         {
             maxAge: 9000000000,
             httpOnly: true,
@@ -17,7 +17,7 @@ app.get('/b', function (req, res, next) {
         httpOnly: true,
         secure: false // NOT OK
     }
-    res.cookie('name', 'value', options);
+    res.cookie('authKey', 'value', options);
     res.end('ok')
 })
 
@@ -32,16 +32,16 @@ app.get('/c', function (req, res, next) {
 })
 
 const js_cookie = require('js-cookie')
-js_cookie.set('key', 'value', { secure: false }); // NOT OK
-js_cookie.set('key', 'value', { secure: true }); // OK
+js_cookie.set('authKey', 'value', { secure: false }); // NOT OK
+js_cookie.set('authKey', 'value', { secure: true }); // OK
 
 const http = require('http');
 
 function test1() {
     const server = http.createServer((req, res) => {
         res.setHeader('Content-Type', 'text/html');
         // BAD
-        res.setHeader("Set-Cookie", "type=ninja");
+        res.setHeader("Set-Cookie", "authKey=ninja");
         res.writeHead(200, { 'Content-Type': 'text/plain' });
         res.end('ok');
     });
@@ -60,8 +60,8 @@ function test2() {
 function test3() {
     const server = http.createServer((req, res) => {
         res.setHeader('Content-Type', 'text/html');
-        // BAD
-        res.setHeader("Set-Cookie", ["type=ninja", "language=javascript"]);
+        // BAD (and good, TODO: Move to separate lines)
+        res.setHeader("Set-Cookie", ["authKey=ninja", "language=javascript"]);
         res.writeHead(200, { 'Content-Type': 'text/plain' });
         res.end('ok');
     });
@@ -90,8 +90,8 @@ function test5() {
 function test6() {
     const server = http.createServer((req, res) => {
         res.setHeader('Content-Type', 'text/html');
-        // BAD
-        res.setHeader("Set-Cookie", ["type=ninja; secure", "language=javascript"]);
+        // BAD (and good. TODO: Move to separate lines)
+        res.setHeader("Set-Cookie", ["type=ninja; secure", "authKey=foo"]);
         res.writeHead(200, { 'Content-Type': 'text/plain' });
         res.end('ok');
     });