update tests, updated qldoc and examples, upgrade all libraries to path-problem, update jsonwebtoken source and sinks

Author: am0o0

javascript/ql/src/Security/CWE-347-noVerification/Example.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/Security/CWE-347-noVerification/Examples/Bad.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/Security/CWE-347-noVerification/Examples/Good.js

@@ -15,14 +15,35 @@ import DataFlow::PathGraph
 
 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()
+  )
 }
 
 DataFlow::Node verifiedDecode() {
-  result = API::moduleImport("jsonwebtoken").getMember("verify").getParameter(0).asSink()
+  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()
+  )
 }
 
 class Configuration extends TaintTracking::Configuration {
-  Configuration() { this = "jsonwebtoken constant secret key" }
+  Configuration() { this = "jsonwebtoken without any signature verification" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
@@ -37,11 +58,8 @@ from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
 where
   cfg.hasFlowPath(source, sink) and
   sink.getNode() = unverifiedDecode() and
-  not isSafe(source.getNode())
+  not exists(Configuration cfg2 |
+    cfg2.hasFlowPath(source, any(DataFlow::SinkPathNode n | n.getNode() = verifiedDecode()))
+  )
 select source.getNode(), source, sink, "Decoding JWT $@.", sink.getNode(),
   "without signature verification"
-
-// Holds if the source has a flow to a JWT decoding function with signature verification
-predicate isSafe(DataFlow::Node source) {
-  exists(Configuration cfg | cfg.hasFlow(source, verifiedDecode()))
-}

javascript/ql/src/Security/CWE-347-noVerification/JsonWebToken.ql

@@ -1,7 +1,7 @@
 /**
  * @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 problem
+ * @kind path-problem
  * @problem.severity error
  * @security-severity 8.0
  * @precision high
@@ -11,7 +11,34 @@
  */
 
 import javascript
+import DataFlow::PathGraph
 
-from DataFlow::Node sink
-where sink = API::moduleImport("jose").getMember("decodeJwt").getParameter(0).asSink()
-select sink, "This Token is Decoded in without signature validation"
+DataFlow::Node unverifiedDecode() {
+  result = API::moduleImport("jose").getMember("decodeJwt").getParameter(0).asSink()
+}
+
+DataFlow::Node verifiedDecode() {
+  result = API::moduleImport("jose").getMember("jwtVerify").getParameter(0).asSink()
+}
+
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "jsonwebtoken without any signature verification" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink = unverifiedDecode()
+    or
+    sink = verifiedDecode()
+  }
+}
+
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where
+  cfg.hasFlowPath(source, sink) and
+  sink.getNode() = unverifiedDecode() and
+  not exists(Configuration cfg2 |
+    cfg2.hasFlowPath(source, any(DataFlow::SinkPathNode n | n.getNode() = verifiedDecode()))
+  )
+select source.getNode(), source, sink, "Decoding JWT $@.", sink.getNode(),
+  "without signature verification"

javascript/ql/src/Security/CWE-347-noVerification/jose.ql

@@ -1,7 +1,7 @@
 /**
  * @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 problem
+ * @kind path-problem
  * @problem.severity error
  * @security-severity 8.0
  * @precision high
@@ -11,9 +11,21 @@
  */
 
 import javascript
+import DataFlow::PathGraph
 
-from DataFlow::Node sink
-where
-  // I must somehow check that following is implemented in server not browser
-  sink = API::moduleImport("jwt-decode").getParameter(0).asSink()
-select sink, "This Token is Decoded in without signature validation"
+DataFlow::Node unverifiedDecode() {
+  result = API::moduleImport("jwt-decode").getParameter(0).asSink()
+}
+
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "jsonwebtoken without any signature verification" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink = unverifiedDecode() }
+}
+
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasFlowPath(source, sink)
+select source.getNode(), source, sink, "Decoding JWT $@.", sink.getNode(),
+  "without signature verification"

javascript/ql/src/Security/CWE-347-noVerification/jwtDecode.ql

@@ -19,15 +19,18 @@
      <example>
 
           <p>
-               The following code you can see an Example from a popular Library.
+               In the following code, you can see the proper usage of the most popular JWT libraries.
           </p>
+          <sample src="Examples/Good.js" />
 
-          <sample src="Example.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 had not been verified</a>
+               <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>
 

javascript/ql/src/Security/CWE-347-noVerification/jwtNoVerification.qhelp

@@ -1,21 +1,53 @@
 /**
  * @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 problem
+ * @kind path-problem
  * @problem.severity error
  * @security-severity 8.0
  * @precision high
- * @id js/jwt-missing-verification
+ * @id js/jwt-missing-verification-jwt-simple
  * @tags security
  *       external/cwe/cwe-347
  */
 
 import javascript
+import DataFlow::PathGraph
 
-from DataFlow::Node sink
-where
+DataFlow::Node unverifiedDecode() {
   exists(API::Node n | n = API::moduleImport("jwt-simple").getMember("decode") |
     n.getParameter(2).asSink().asExpr() = any(BoolLiteral b | b.getBoolValue() = true) and
-    sink = n.getParameter(0).asSink()
+    result = n.getParameter(0).asSink()
+  )
+}
+
+DataFlow::Node verifiedDecode() {
+  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()
+  )
+}
+
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "jsonwebtoken without any signature verification" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink = unverifiedDecode()
+    or
+    sink = verifiedDecode()
+  }
+}
+
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where
+  cfg.hasFlowPath(source, sink) and
+  sink.getNode() = unverifiedDecode() and
+  not exists(Configuration cfg2 |
+    cfg2.hasFlowPath(source, any(DataFlow::SinkPathNode n | n.getNode() = verifiedDecode()))
   )
-select sink, "This Token is Decoded in without signature validation"
+select source.getNode(), source, sink, "Decoding JWT $@.", sink.getNode(),
+  "without signature verification"