Merge pull request github#7740 from erik-krogh/CWE-347

JS: promote the js/jwt-missing-verification query out of experimental

Author: erik-krogh

javascript/ql/src/Security/CWE-347/MissingJWTKeyVerification.qhelp

@@ -0,0 +1,42 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd"> <qhelp>
+
+<overview>
+<p>
+Applications decoding JSON Web Tokens (JWT) may be misconfigured due to the <code>None</code> algorithm.
+</p>
+<p>
+The <code>None</code> algorithm is selected by calling the <code>verify()</code> function with a falsy value
+instead of a cryptographic secret or key. The <code>None</code> algorithm disables the integrity enforcement of
+a JWT payload and may allow a malicious actor to make unintended changes to a JWT payload leading
+to critical security issues like privilege escalation.
+</p>
+
+</overview>
+
+<recommendation>
+<p>
+Calls to <code>verify()</code> functions should use a cryptographic secret or key to decode JWT payloads.
+</p>
+</recommendation>
+
+<example>
+<p>
+In the example below, <code>false</code> is used to disable the integrity enforcement of a JWT payload. 
+This may allow a malicious actor to make changes to a JWT payload.
+</p>
+
+<sample src="examples/missing-key-verification-bad.js" />
+
+<p>
+The following code fixes the problem by using a cryptographic secret or key to decode JWT payloads.
+</p>
+
+<sample src="examples/missing-key-verification-good.js" />
+
+</example>
+
+<references>
+<li>Auth0 Blog: <a href="https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/#Meet-the--None--Algorithm">Meet the "None" Algorithm</a>.</li>
+
+</references>
+</qhelp>

javascript/ql/src/experimental/Security/CWE-347/JWTMissingSecretOrPublicKeyVerification.ql renamed to javascript/ql/src/Security/CWE-347/MissingJWTKeyVerification.ql

@@ -10,12 +10,11 @@
  */
 
 import javascript
-import DataFlow
 import semmle.javascript.RestrictedLocations
 
-from CallNode call
+from DataFlow::CallNode call
 where
-  call = moduleMember("jsonwebtoken", "verify").getACall() and
-  unique(boolean b | b = call.getArgument(1).analyze().getABooleanValue()) = false
-select call.asExpr().(FirstLineOf),
-  "does not verify the JWT payload with a cryptographic secret or public key."
+  call = DataFlow::moduleMember("jsonwebtoken", "verify").getACall() and
+  call.getArgument(1).analyze().getTheBooleanValue() = false
+select call.getArgument(1),
+  "This argument disables the integrity enforcement of the token verification."

javascript/ql/src/Security/CWE-347/examples/missing-key-verification-bad.js

@@ -0,0 +1,6 @@
+const jwt = require("jsonwebtoken");
+
+const secret = "my-secret-key";
+
+var token = jwt.sign({ foo: 'bar' }, secret, { algorithm: "none" })
+jwt.verify(token, false, { algorithms: ["HS256", "none"] })

javascript/ql/src/Security/CWE-347/examples/missing-key-verification-good.js

@@ -0,0 +1,7 @@
+
+const jwt = require("jsonwebtoken");
+
+const secret = "my-secret-key";
+
+var token = jwt.sign({ foo: 'bar' }, secret, { algorithm: "HS256" }) 
+jwt.verify(token, secret, { algorithms: ["HS256", "none"] })

javascript/ql/src/change-notes/2022-01-25-missing-jwt-verification.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* A new query `js/jwt-missing-verification` has been added. The query detects applications that don't verify JWT tokens.

javascript/ql/src/experimental/Security/CWE-347/JWTMissingSecretOrPublicKeyVerification.help

@@ -0,0 +1,3 @@
+| bad-jwt.js:10:19:10:20 | "" | This argument disables the integrity enforcement of the token verification. |
+| bad-jwt.js:11:19:11:27 | undefined | This argument disables the integrity enforcement of the token verification. |
+| bad-jwt.js:12:19:12:23 | false | This argument disables the integrity enforcement of the token verification. |

javascript/ql/test/query-tests/Security/CWE-347/MissingJWTKeyVerification.expected

@@ -0,0 +1 @@
+Security/CWE-347/MissingJWTKeyVerification.ql

javascript/ql/test/query-tests/Security/CWE-347/MissingJWTKeyVerification.qlref

@@ -1,11 +1,12 @@
 const jwt = require("jsonwebtoken");
 
-const secret = "buybtc";
-// #1
-var token = jwt.sign({ foo: 'bar' }, secret, { algorithm: "HS256" }) // alg:HS256
-jwt.verify(token, secret, { algorithms: ["HS256", "none"] }) // pass
-// #2
-var token = jwt.sign({ foo: 'bar' }, secret, { algorithm: "none" }) // alg:none (unsafe)
-jwt.verify(token, "", { algorithms: ["HS256", "none"] }) // detected
-jwt.verify(token, undefined, { algorithms: ["HS256", "none"] }) // detected
-jwt.verify(token, false, { algorithms: ["HS256", "none"] }) // detected
+const secret = "my-secret-key";
+
+var token = jwt.sign({ foo: 'bar' }, secret, { algorithm: "HS256" }) 
+jwt.verify(token, secret, { algorithms: ["HS256", "none"] }) // OK
+
+
+var token = jwt.sign({ foo: 'bar' }, secret, { algorithm: "none" })
+jwt.verify(token, "", { algorithms: ["HS256", "none"] }) // NOT OK
+jwt.verify(token, undefined, { algorithms: ["HS256", "none"] }) // NOT OK
+jwt.verify(token, false, { algorithms: ["HS256", "none"] }) // NOT OK