Merge pull request github#14088 from am0o0/amammad-js-JWT

JS: decoding JWT without signature verification

Author: erik-krogh

javascript/ql/src/experimental/Security/CWE-347/Examples/Bad.js

@@ -0,0 +1,39 @@
+const express = require('express')
+const app = express()
+const jwtJsonwebtoken = require('jsonwebtoken');
+const jwt_decode = require('jwt-decode');
+const jwt_simple = require('jwt-simple');
+const jose = require('jose')
+const port = 3000
+
+function getSecret() {
+    return "A Safe generated random key"
+}
+app.get('/jose', (req, res) => {
+    const UserToken = req.headers.authorization;
+    // BAD: no signature verification
+    jose.decodeJwt(UserToken)
+})
+
+app.get('/jwtDecode', (req, res) => {
+    const UserToken = req.headers.authorization;
+    // BAD: no signature verification
+    jwt_decode(UserToken)
+})
+
+app.get('/jwtSimple', (req, res) => {
+    const UserToken = req.headers.authorization;
+    // jwt.decode(token, key, noVerify, algorithm)
+    // BAD: no signature verification
+    jwt_simple.decode(UserToken, getSecret(), true);
+})
+
+app.get('/jwtJsonwebtoken', (req, res) => {
+    const UserToken = req.headers.authorization;
+    // BAD: no signature verification
+    jwtJsonwebtoken.decode(UserToken)
+})
+
+app.listen(port, () => {
+    console.log(`Example app listening on port ${port}`)
+})

javascript/ql/src/experimental/Security/CWE-347/Examples/Good.js

@@ -0,0 +1,56 @@
+const express = require('express')
+const app = express()
+const jwtJsonwebtoken = require('jsonwebtoken');
+const jwt_decode = require('jwt-decode');
+const jwt_simple = require('jwt-simple');
+const jose = require('jose')
+const port = 3000
+
+function getSecret() {
+    return "A Safe generated random key"
+}
+
+app.get('/jose1', async (req, res) => {
+    const UserToken = req.headers.authorization;
+    // GOOD: with signature verification
+    await jose.jwtVerify(UserToken, new TextEncoder().encode(getSecret()))
+})
+
+app.get('/jose2', async (req, res) => {
+    const UserToken = req.headers.authorization;
+    // GOOD: first without signature verification then with signature verification for same UserToken
+    jose.decodeJwt(UserToken)
+    await jose.jwtVerify(UserToken, new TextEncoder().encode(getSecret()))
+})
+
+app.get('/jwtSimple1', (req, res) => {
+    const UserToken = req.headers.authorization;
+    // GOOD: first without signature verification then with signature verification for same UserToken
+    jwt_simple.decode(UserToken, getSecret(), false);
+    jwt_simple.decode(UserToken, getSecret());
+})
+
+app.get('/jwtSimple2', (req, res) => {
+    const UserToken = req.headers.authorization;
+    // GOOD: with signature verification
+    jwt_simple.decode(UserToken, getSecret(), true);
+    jwt_simple.decode(UserToken, getSecret());
+})
+
+app.get('/jwtJsonwebtoken1', (req, res) => {
+    const UserToken = req.headers.authorization;
+    // GOOD: with signature verification
+    jwtJsonwebtoken.verify(UserToken, getSecret())
+})
+
+app.get('/jwtJsonwebtoken2', (req, res) => {
+    const UserToken = req.headers.authorization;
+    // GOOD: first without signature verification then with signature verification for same UserToken
+    jwtJsonwebtoken.decode(UserToken)
+    jwtJsonwebtoken.verify(UserToken, getSecret())
+})
+
+
+app.listen(port, () => {
+    console.log(`Example app listening on port ${port}`)
+})

javascript/ql/src/experimental/Security/CWE-347/JWT.qll

@@ -0,0 +1,54 @@
+import javascript
+
+DataFlow::Node unverifiedDecode() {
+  result = API::moduleImport("jsonwebtoken").getMember("decode").getParameter(0).asSink()
+  or
+  exists(API::Node verify | verify = API::moduleImport("jsonwebtoken").getMember("verify") |
+    verify
+        .getParameter(2)
+        .getMember("algorithms")
+        .getUnknownMember()
+        .asSink()
+        .mayHaveStringValue("none") and
+    result = verify.getParameter(0).asSink()
+  )
+  or
+  // jwt-simple
+  exists(API::Node n | n = API::moduleImport("jwt-simple").getMember("decode") |
+    n.getParameter(2).asSink().asExpr() = any(BoolLiteral b | b.getBoolValue() = true) and
+    result = n.getParameter(0).asSink()
+  )
+  or
+  // jwt-decode
+  result = API::moduleImport("jwt-decode").getParameter(0).asSink()
+  or
+  //jose
+  result = API::moduleImport("jose").getMember("decodeJwt").getParameter(0).asSink()
+}
+
+DataFlow::Node verifiedDecode() {
+  exists(API::Node verify | verify = API::moduleImport("jsonwebtoken").getMember("verify") |
+    (
+      not verify
+          .getParameter(2)
+          .getMember("algorithms")
+          .getUnknownMember()
+          .asSink()
+          .mayHaveStringValue("none") or
+      not exists(verify.getParameter(2).getMember("algorithms"))
+    ) and
+    result = verify.getParameter(0).asSink()
+  )
+  or
+  // jwt-simple
+  exists(API::Node n | n = API::moduleImport("jwt-simple").getMember("decode") |
+    (
+      n.getParameter(2).asSink().asExpr() = any(BoolLiteral b | b.getBoolValue() = false) or
+      not exists(n.getParameter(2))
+    ) and
+    result = n.getParameter(0).asSink()
+    or
+    //jose
+    result = API::moduleImport("jose").getMember("jwtVerify").getParameter(0).asSink()
+  )
+}

javascript/ql/src/experimental/Security/CWE-347/decodeJwtWithoutVerification.qhelp

@@ -0,0 +1,37 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+     <overview>
+          <p>
+               A JSON Web Token (JWT) is used for authenticating and managing users in an application.
+          </p>
+          <p>
+               Only Decoding JWTs without checking if they have a valid signature or not can lead to security vulnerabilities.
+          </p>
+
+     </overview>
+     <recommendation>
+
+          <p>
+               Don't use methods that only decode JWT, Instead use methods that verify the signature of JWT.
+          </p>
+
+     </recommendation>
+     <example>
+
+          <p>
+               In the following code, you can see the proper usage of the most popular JWT libraries.
+          </p>
+          <sample src="Examples/Good.js" />
+
+          <p>
+               In the following code, you can see the improper usage of the most popular JWT libraries.
+          </p>
+          <sample src="Examples/Bad.js" />
+     </example>
+     <references>
+          <li>
+               <a href="https://www.ghostccamm.com/blog/multi_strapi_vulns/#cve-2023-22893-authentication-bypass-for-aws-cognito-login-provider-in-strapi-versions-456">JWT claim has not been verified</a>
+          </li>
+     </references>
+
+</qhelp>

javascript/ql/src/experimental/Security/CWE-347/decodeJwtWithoutVerification.ql

@@ -0,0 +1,40 @@
+/**
+ * @name JWT missing secret or public key verification
+ * @description The application does not verify the JWT payload with a cryptographic secret or public key.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 8.0
+ * @precision high
+ * @id js/decode-jwt-without-verification
+ * @tags security
+ *       external/cwe/cwe-347
+ */
+
+import javascript
+import DataFlow::PathGraph
+import JWT
+
+class ConfigurationUnverifiedDecode extends TaintTracking::Configuration {
+  ConfigurationUnverifiedDecode() { this = "jsonwebtoken without any signature verification" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink = unverifiedDecode() }
+}
+
+class ConfigurationVerifiedDecode extends TaintTracking::Configuration {
+  ConfigurationVerifiedDecode() { this = "jsonwebtoken with signature verification" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink = verifiedDecode() }
+}
+
+from ConfigurationUnverifiedDecode cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where
+  cfg.hasFlowPath(source, sink) and
+  not exists(ConfigurationVerifiedDecode cfg2 |
+    cfg2.hasFlowPath(any(DataFlow::PathNode p | p.getNode() = source.getNode()), _)
+  )
+select source.getNode(), source, sink, "Decoding JWT $@.", sink.getNode(),
+  "without signature verification"

javascript/ql/src/experimental/Security/CWE-347/decodeJwtWithoutVerificationLocalSource.ql

@@ -0,0 +1,55 @@
+/**
+ * @name JWT missing secret or public key verification
+ * @description The application does not verify the JWT payload with a cryptographic secret or public key.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 8.0
+ * @precision high
+ * @id js/decode-jwt-without-verification-local-source
+ * @tags security
+ *       external/cwe/cwe-347
+ */
+
+import javascript
+import DataFlow::PathGraph
+import JWT
+
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "jsonwebtoken without any signature verification" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source = [unverifiedDecode(), verifiedDecode()].getALocalSource()
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink = unverifiedDecode()
+    or
+    sink = verifiedDecode()
+  }
+}
+
+/** Holds if `source` flows to the first parameter of jsonwebtoken.verify */
+predicate isSafe(Configuration cfg, DataFlow::Node source) {
+  exists(DataFlow::Node sink |
+    cfg.hasFlow(source, sink) and
+    sink = verifiedDecode()
+  )
+}
+
+/**
+ * Holds if:
+ * - `source` does not flow to the first parameter of `jsonwebtoken.verify`, and
+ * - `source` flows to the first parameter of `jsonwebtoken.decode`
+ */
+predicate isVulnerable(Configuration cfg, DataFlow::Node source, DataFlow::Node sink) {
+  not isSafe(cfg, source) and // i.e., source does not flow to a verify call
+  cfg.hasFlow(source, sink) and // but it does flow to something else
+  sink = unverifiedDecode() // and that something else is a call to decode.
+}
+
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where
+  cfg.hasFlowPath(source, sink) and
+  isVulnerable(cfg, source.getNode(), sink.getNode())
+select source.getNode(), source, sink, "Decoding JWT $@.", sink.getNode(),
+  "without signature verification"

javascript/ql/test/experimental/Security/CWE-347/localsource/JsonWebToken.js

@@ -0,0 +1,48 @@
+const express = require('express')
+const jwtJsonwebtoken = require('jsonwebtoken');
+
+function getSecret() {
+    return "A Safe generated random key"
+}
+
+function aJWT() {
+    return "A JWT provided by user"
+}
+
+(function () {
+    const UserToken = aJwt()
+
+    // BAD: no signature verification
+    jwtJsonwebtoken.decode(UserToken) // NOT OK
+})();
+
+(function () {
+    const UserToken = aJwt()
+
+    // BAD: no signature verification
+    jwtJsonwebtoken.decode(UserToken) // NOT OK
+    jwtJsonwebtoken.verify(UserToken, getSecret(), { algorithms: ["HS256", "none"] }) // NOT OK
+})();
+
+(function () {
+    const UserToken = aJwt()
+
+    // GOOD: with signature verification
+    jwtJsonwebtoken.verify(UserToken, getSecret())  // OK
+})();
+
+(function () {
+    const UserToken = aJwt()
+
+    // GOOD: first without signature verification then with signature verification for same UserToken
+    jwtJsonwebtoken.decode(UserToken)  // OK
+    jwtJsonwebtoken.verify(UserToken, getSecret())  // OK
+})();
+
+(function () {
+    const UserToken = aJwt()
+
+    // GOOD: first without signature verification then with signature verification for same UserToken
+    jwtJsonwebtoken.decode(UserToken) // OK
+    jwtJsonwebtoken.verify(UserToken, getSecret(), { algorithms: ["HS256"] }) // OK
+})();