Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
65b9774
V1
am0o0 Aug 29, 2023
4f04dc8
add test cases
am0o0 Aug 29, 2023
3f64cc8
fix qhelps
am0o0 Aug 29, 2023
7a577dd
change Source to ConstantString, it seems that we have some duplicate…
am0o0 Aug 30, 2023
7891e64
add sanitizers to hardcoded query
am0o0 Oct 15, 2023
ee4d87b
remove hardcoded JWT secret-key query
am0o0 Oct 19, 2023
9da815a
move to new CWE-321 directory, make saparate query files for each JWT…
am0o0 Nov 2, 2023
faa483a
move to CWE-347, update comments of tests
am0o0 Nov 2, 2023
a9c8bc0
delete CWE-321
am0o0 Nov 2, 2023
0652afc
update tests, updated qldoc and examples, upgrade all libraries to pa…
am0o0 Nov 7, 2023
5cc4206
add a temporary Query file to demonstrate unsuccessful usage of two D…
am0o0 Nov 22, 2023
48a9b10
add query to detect strapi CVe too
am0o0 Nov 24, 2023
c470c07
move to experimental
am0o0 May 21, 2024
0895f7d
update qlref files
am0o0 May 21, 2024
f905ac1
add jsonWebToken library file to remove duplicate predicate declrations
am0o0 May 25, 2024
4af4040
change duplicate query IDs
am0o0 May 25, 2024
d2d945c
merge all JWT pkgs into one
am0o0 May 25, 2024
f1533f4
change query files name
am0o0 May 25, 2024
76beffb
change dir name
am0o0 May 25, 2024
300c82a
use Verification instead of validation in files name
am0o0 May 25, 2024
b397f57
change queries id according to new naming
am0o0 May 25, 2024
8fde8c2
change test dir name
am0o0 May 25, 2024
14daf58
update tests, add test cases for query with local sources
am0o0 May 25, 2024
ea05b29
update expected test files
am0o0 May 25, 2024
af016f9
Merge branch 'github:main' into amammad-js-JWT
am0o0 Jun 6, 2024
12df7de
Merge branch 'amammad-js-JWT' of https://github.com/amammad/codeql in…
am0o0 Jun 6, 2024
b9e3b33
update the remote flow based query thanks to @erik-krogh, update test…
am0o0 Jun 7, 2024
1033bf9
remove unused imports from javascript test cases
am0o0 Jun 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions javascript/ql/src/experimental/Security/CWE-347/Examples/Bad.js
Original file line number Diff line number Diff line change
@@ -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}`)
})
Original file line number Diff line number Diff line change
@@ -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}`)
})
54 changes: 54 additions & 0 deletions javascript/ql/src/experimental/Security/CWE-347/JWT.qll
Original file line number Diff line number Diff line change
@@ -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()
)
}
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -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
})();
Loading