Skip to content

Commit 9da815a

Browse files
committed
move to new CWE-321 directory, make saparate query files for each JWT pkg, create a path query for jsonwebtoken package which is not work correctly
1 parent ee4d87b commit 9da815a

File tree

7 files changed

+188
-32
lines changed

7 files changed

+188
-32
lines changed
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/**
2+
* @name JWT missing secret or public key verification
3+
* @description The application does not verify the JWT payload with a cryptographic secret or public key.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 9.0
7+
* @precision high
8+
* @id js/jwt-missing-verification
9+
* @tags security
10+
* external/cwe/cwe-321
11+
*/
12+
13+
import javascript
14+
import DataFlow::PathGraph
15+
16+
DataFlow::Node unverifiedDecode() {
17+
result = API::moduleImport("jsonwebtoken").getMember("decode").getParameter(0).asSink()
18+
}
19+
20+
DataFlow::Node verifiedDecode() {
21+
result = API::moduleImport("jsonwebtoken").getMember("verify").getParameter(0).asSink()
22+
}
23+
24+
class Configuration extends TaintTracking::Configuration {
25+
Configuration() { this = "jsonwebtoken constant secret key" }
26+
27+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
28+
29+
override predicate isSink(DataFlow::Node sink) {
30+
sink = unverifiedDecode()
31+
or
32+
sink = verifiedDecode()
33+
}
34+
}
35+
36+
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
37+
where
38+
cfg.hasFlowPath(source, sink) and
39+
sink.getNode() = unverifiedDecode() and
40+
not isSafe(source.getNode())
41+
select source.getNode(), source, sink, "Decoding JWT $@.", sink.getNode(),
42+
"without signature verification"
43+
44+
// Holds if the source has a flow to a JWT decoding function with signature verification
45+
predicate isSafe(DataFlow::Node source) {
46+
exists(Configuration cfg | cfg.hasFlow(source, verifiedDecode()))
47+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/**
2+
* @name JWT missing secret or public key verification
3+
* @description The application does not verify the JWT payload with a cryptographic secret or public key.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @security-severity 7.0
7+
* @precision high
8+
* @id js/jwt-missing-verification
9+
* @tags security
10+
* external/cwe/cwe-347
11+
*/
12+
13+
import javascript
14+
15+
from DataFlow::Node sink
16+
where sink = API::moduleImport("jose").getMember("decodeJwt").getParameter(0).asSink()
17+
select sink, "This Token is Decoded in without signature validation"
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @name JWT missing secret or public key verification
3+
* @description The application does not verify the JWT payload with a cryptographic secret or public key.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @security-severity 7.0
7+
* @precision high
8+
* @id js/jwt-missing-verification
9+
* @tags security
10+
* external/cwe/cwe-347
11+
*/
12+
13+
import javascript
14+
15+
from DataFlow::Node sink
16+
where
17+
// I must somehow check that following is implemented in server not browser
18+
sink = API::moduleImport("jwt-decode").getParameter(0).asSink()
19+
select sink, "This Token is Decoded in without signature validation"

javascript/ql/src/Security/CWE-321-noVerification/jwtNoVerification.ql renamed to javascript/ql/src/Security/CWE-321-noVerification/jwtSimple.ql

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,8 @@ import javascript
1414

1515
from DataFlow::Node sink
1616
where
17-
sink = API::moduleImport("jsonwebtoken").getMember("decode").getParameter(0).asSink()
18-
or
19-
sink = API::moduleImport("jwt-decode").getParameter(0).asSink()
20-
or
21-
sink = API::moduleImport("jose").getMember("decodeJwt").getParameter(0).asSink()
22-
or
2317
exists(API::Node n | n = API::moduleImport("jwt-simple").getMember("decode") |
2418
n.getParameter(2).asSink().asExpr() = any(BoolLiteral b | b.getBoolValue() = true) and
2519
sink = n.getParameter(0).asSink()
2620
)
27-
select sink, "This Token is Decoded in without signature validatoin"
21+
select sink, "This Token is Decoded in without signature validation"

javascript/ql/test/query-tests/Security/CWE-321-noVerification/NoVerification.js

Lines changed: 65 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,90 @@
11
const express = require('express')
22
const app = express()
33
const jwtJsonwebtoken = require('jsonwebtoken');
4-
const {getSecret} = require('./Config.js');
4+
const { getSecret } = require('./Config.js');
55
const jwt_decode = require('jwt-decode');
66
const jwt_simple = require('jwt-simple');
77
const jose = require('jose')
88
const port = 3000
99

1010
async function startSymmetric(token) {
11-
const {payload, protectedHeader} = await jose.jwtVerify(token, new TextEncoder().encode(getSecret()))
11+
const { payload, protectedHeader } = await jose.jwtVerify(token, new TextEncoder().encode(getSecret()))
1212
return {
1313
payload, protectedHeader
1414
}
1515
}
1616

17-
app.get('/', (req, res) => {
17+
app.get('/jose', (req, res) => {
1818
const UserToken = req.headers.authorization;
19-
// BAD: no verification
20-
jwtJsonwebtoken.decode(UserToken)
21-
jwtJsonwebtoken.verify(UserToken, false, {algorithms: ["HS256", "none"]})
22-
// GOOD: use verify alone or use as a check,
23-
// sometimes it seems some coders use both for same token
24-
const UserToken2 = req.headers.authorization;
25-
jwtJsonwebtoken.decode(UserToken2)
26-
jwtJsonwebtoken.verify(UserToken2, getSecret())
27-
// jwt-decode
28-
// BAD: no verification
29-
jwt_decode(UserToken)
19+
3020
// jose
31-
// BAD: no verification
21+
// BAD: no signature verification
3222
jose.decodeJwt(UserToken)
33-
// GOOD
23+
// GOOD: with signature verification
3424
startSymmetric(UserToken).then(result => console.log(result))
25+
26+
27+
})
28+
29+
30+
app.get('/jwtDecode', (req, res) => {
31+
const UserToken = req.headers.authorization;
32+
33+
// jwt-decode
34+
// BAD: no signature verification
35+
jwt_decode(UserToken)
36+
})
37+
38+
app.get('/jwtSimple', (req, res) => {
39+
const UserToken = req.headers.authorization;
40+
3541
// jwt-simple
36-
// no verification
42+
// jwt.decode(token, key, noVerify, algorithm)
43+
// BAD: no signature verification
3744
jwt_simple.decode(UserToken, getSecret(), true);
38-
// GOOD
45+
})
46+
47+
app.get('/jwtSimple2', (req, res) => {
48+
const UserToken = req.headers.authorization;
49+
50+
// jwt-simple
51+
// jwt.decode(token, key, noVerify, algorithm)
52+
// GOOD: with signature verification
3953
jwt_simple.decode(UserToken, getSecret(), false);
4054
jwt_simple.decode(UserToken, getSecret());
41-
res.send('Hello World!')
55+
})
56+
57+
app.get('/jwtSimple3', (req, res) => {
58+
const UserToken = req.headers.authorization;
59+
60+
// jwt-simple
61+
// jwt.decode(token, key, noVerify, algorithm)
62+
// GOOD: first decode without signature verification and then verify the signature later
63+
jwt_simple.decode(UserToken, getSecret(), true);
64+
jwt_simple.decode(UserToken, getSecret());
65+
})
66+
67+
app.get('/jwtJsonwebtoken', (req, res) => {
68+
const UserToken = req.headers.authorization;
69+
70+
// BAD: no signature verification
71+
jwtJsonwebtoken.decode(UserToken)
72+
jwtJsonwebtoken.verify(UserToken, false, { algorithms: ["HS256", "none"] })
73+
})
74+
75+
app.get('/jwtJsonwebtoken2', (req, res) => {
76+
const UserToken = req.headers.authorization;
77+
78+
// GOOD: with signature verification
79+
jwtJsonwebtoken.verify(UserToken, getSecret())
80+
})
81+
82+
app.get('/jwtJsonwebtoken3', (req, res) => {
83+
const UserToken = req.headers.authorization;
84+
85+
// GOOD: first decode without signature verification and then verify the signature later
86+
jwtJsonwebtoken.decode(UserToken)
87+
jwtJsonwebtoken.verify(UserToken, getSecret())
4288
})
4389

4490
app.listen(port, () => {
Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,38 @@
1-
| NoVerification.js:20:28:20:36 | UserToken | This Token is Decoded in without signature validatoin |
2-
| NoVerification.js:25:28:25:37 | UserToken2 | This Token is Decoded in without signature validatoin |
3-
| NoVerification.js:29:16:29:24 | UserToken | This Token is Decoded in without signature validatoin |
4-
| NoVerification.js:32:20:32:28 | UserToken | This Token is Decoded in without signature validatoin |
5-
| NoVerification.js:37:23:37:31 | UserToken | This Token is Decoded in without signature validatoin |
1+
nodes
2+
| NoVerification.js:68:11:68:47 | UserToken |
3+
| NoVerification.js:68:23:68:47 | req.hea ... ization |
4+
| NoVerification.js:68:23:68:47 | req.hea ... ization |
5+
| NoVerification.js:71:28:71:36 | UserToken |
6+
| NoVerification.js:71:28:71:36 | UserToken |
7+
| NoVerification.js:72:28:72:36 | UserToken |
8+
| NoVerification.js:72:28:72:36 | UserToken |
9+
| NoVerification.js:76:11:76:47 | UserToken |
10+
| NoVerification.js:76:23:76:47 | req.hea ... ization |
11+
| NoVerification.js:76:23:76:47 | req.hea ... ization |
12+
| NoVerification.js:79:28:79:36 | UserToken |
13+
| NoVerification.js:79:28:79:36 | UserToken |
14+
| NoVerification.js:83:11:83:47 | UserToken |
15+
| NoVerification.js:83:23:83:47 | req.hea ... ization |
16+
| NoVerification.js:83:23:83:47 | req.hea ... ization |
17+
| NoVerification.js:86:28:86:36 | UserToken |
18+
| NoVerification.js:86:28:86:36 | UserToken |
19+
| NoVerification.js:87:28:87:36 | UserToken |
20+
| NoVerification.js:87:28:87:36 | UserToken |
21+
edges
22+
| NoVerification.js:68:11:68:47 | UserToken | NoVerification.js:71:28:71:36 | UserToken |
23+
| NoVerification.js:68:11:68:47 | UserToken | NoVerification.js:71:28:71:36 | UserToken |
24+
| NoVerification.js:68:11:68:47 | UserToken | NoVerification.js:72:28:72:36 | UserToken |
25+
| NoVerification.js:68:11:68:47 | UserToken | NoVerification.js:72:28:72:36 | UserToken |
26+
| NoVerification.js:68:23:68:47 | req.hea ... ization | NoVerification.js:68:11:68:47 | UserToken |
27+
| NoVerification.js:68:23:68:47 | req.hea ... ization | NoVerification.js:68:11:68:47 | UserToken |
28+
| NoVerification.js:76:11:76:47 | UserToken | NoVerification.js:79:28:79:36 | UserToken |
29+
| NoVerification.js:76:11:76:47 | UserToken | NoVerification.js:79:28:79:36 | UserToken |
30+
| NoVerification.js:76:23:76:47 | req.hea ... ization | NoVerification.js:76:11:76:47 | UserToken |
31+
| NoVerification.js:76:23:76:47 | req.hea ... ization | NoVerification.js:76:11:76:47 | UserToken |
32+
| NoVerification.js:83:11:83:47 | UserToken | NoVerification.js:86:28:86:36 | UserToken |
33+
| NoVerification.js:83:11:83:47 | UserToken | NoVerification.js:86:28:86:36 | UserToken |
34+
| NoVerification.js:83:11:83:47 | UserToken | NoVerification.js:87:28:87:36 | UserToken |
35+
| NoVerification.js:83:11:83:47 | UserToken | NoVerification.js:87:28:87:36 | UserToken |
36+
| NoVerification.js:83:23:83:47 | req.hea ... ization | NoVerification.js:83:11:83:47 | UserToken |
37+
| NoVerification.js:83:23:83:47 | req.hea ... ization | NoVerification.js:83:11:83:47 | UserToken |
38+
#select
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Security/CWE-321-noVerification/jwtNoVerification.ql
1+
Security/CWE-321-noVerification/JsonWebToken.ql

0 commit comments

Comments
 (0)