Skip to content

Commit b9e3b33

Browse files
am0o0am0o0
committed
update the remote flow based query thanks to @erik-krogh, update tests and separate the local and remote query tests
1 parent 12df7de commit b9e3b33

15 files changed

+326
-436
lines changed

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,24 +14,27 @@ import javascript
1414
import DataFlow::PathGraph
1515
import JWT
1616

17-
class Configuration extends TaintTracking::Configuration {
18-
Configuration() { this = "jsonwebtoken without any signature verification" }
17+
class ConfigurationUnverifiedDecode extends TaintTracking::Configuration {
18+
ConfigurationUnverifiedDecode() { this = "jsonwebtoken without any signature verification" }
1919

2020
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
2121

22-
override predicate isSink(DataFlow::Node sink) {
23-
sink = unverifiedDecode()
24-
or
25-
sink = verifiedDecode()
26-
}
22+
override predicate isSink(DataFlow::Node sink) { sink = unverifiedDecode() }
2723
}
2824

29-
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
25+
class ConfigurationVerifiedDecode extends TaintTracking::Configuration {
26+
ConfigurationVerifiedDecode() { this = "jsonwebtoken with signature verification" }
27+
28+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
29+
30+
override predicate isSink(DataFlow::Node sink) { sink = verifiedDecode() }
31+
}
32+
33+
from ConfigurationUnverifiedDecode cfg, DataFlow::PathNode source, DataFlow::PathNode sink
3034
where
3135
cfg.hasFlowPath(source, sink) and
32-
sink.getNode() = unverifiedDecode() and
33-
not exists(Configuration cfg2 |
34-
cfg2.hasFlowPath(source, any(DataFlow::SinkPathNode n | n.getNode() = verifiedDecode()))
36+
not exists(ConfigurationVerifiedDecode cfg2 |
37+
cfg2.hasFlowPath(any(DataFlow::PathNode p | p.getNode() = source.getNode()), _)
3538
)
3639
select source.getNode(), source, sink, "Decoding JWT $@.", sink.getNode(),
3740
"without signature verification"

‎javascript/ql/src/experimental/Security/CWE-347/decodeJwtWithoutVerificationDoesNotWork.ql

Lines changed: 0 additions & 38 deletions
This file was deleted.

‎javascript/ql/test/experimental/Security/CWE-347/decodeJwtWithoutVerificationLocalSource.expected

Lines changed: 0 additions & 279 deletions
This file was deleted.

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
const express = require('express')
2+
const app = express()
3+
const jwtJsonwebtoken = require('jsonwebtoken');
4+
const jwt_decode = require('jwt-decode');
5+
const jwt_simple = require('jwt-simple');
6+
const jose = require('jose')
7+
const port = 3000
8+
9+
function getSecret() {
10+
return "A Safe generated random key"
11+
}
12+
13+
function aJWT() {
14+
return "A JWT provided by user"
15+
}
16+
17+
(function () {
18+
const UserToken = aJwt()
19+
20+
// BAD: no signature verification
21+
jwtJsonwebtoken.decode(UserToken) // NOT OK
22+
})();
23+
24+
(function () {
25+
const UserToken = aJwt()
26+
27+
// BAD: no signature verification
28+
jwtJsonwebtoken.decode(UserToken) // NOT OK
29+
jwtJsonwebtoken.verify(UserToken, getSecret(), { algorithms: ["HS256", "none"] }) // NOT OK
30+
})();
31+
32+
(function () {
33+
const UserToken = aJwt()
34+
35+
// GOOD: with signature verification
36+
jwtJsonwebtoken.verify(UserToken, getSecret()) // OK
37+
})();
38+
39+
(function () {
40+
const UserToken = aJwt()
41+
42+
// GOOD: first without signature verification then with signature verification for same UserToken
43+
jwtJsonwebtoken.decode(UserToken) // OK
44+
jwtJsonwebtoken.verify(UserToken, getSecret()) // OK
45+
})();
46+
47+
(function () {
48+
const UserToken = aJwt()
49+
50+
// GOOD: first without signature verification then with signature verification for same UserToken
51+
jwtJsonwebtoken.decode(UserToken) // OK
52+
jwtJsonwebtoken.verify(UserToken, getSecret(), { algorithms: ["HS256"] }) // OK
53+
})();

‎javascript/ql/test/experimental/Security/CWE-347/localsource/decodeJwtWithoutVerificationLocalSource.expected

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
nodes
2+
| JsonWebToken.js:18:11:18:28 | UserToken |
3+
| JsonWebToken.js:18:23:18:28 | aJwt() |
4+
| JsonWebToken.js:18:23:18:28 | aJwt() |
5+
| JsonWebToken.js:21:28:21:36 | UserToken |
6+
| JsonWebToken.js:21:28:21:36 | UserToken |
7+
| JsonWebToken.js:25:11:25:28 | UserToken |
8+
| JsonWebToken.js:25:23:25:28 | aJwt() |
9+
| JsonWebToken.js:25:23:25:28 | aJwt() |
10+
| JsonWebToken.js:28:28:28:36 | UserToken |
11+
| JsonWebToken.js:28:28:28:36 | UserToken |
12+
| JsonWebToken.js:29:28:29:36 | UserToken |
13+
| JsonWebToken.js:29:28:29:36 | UserToken |
14+
| JsonWebToken.js:33:11:33:28 | UserToken |
15+
| JsonWebToken.js:33:23:33:28 | aJwt() |
16+
| JsonWebToken.js:33:23:33:28 | aJwt() |
17+
| JsonWebToken.js:36:28:36:36 | UserToken |
18+
| JsonWebToken.js:36:28:36:36 | UserToken |
19+
| JsonWebToken.js:40:11:40:28 | UserToken |
20+
| JsonWebToken.js:40:23:40:28 | aJwt() |
21+
| JsonWebToken.js:40:23:40:28 | aJwt() |
22+
| JsonWebToken.js:43:28:43:36 | UserToken |
23+
| JsonWebToken.js:43:28:43:36 | UserToken |
24+
| JsonWebToken.js:44:28:44:36 | UserToken |
25+
| JsonWebToken.js:44:28:44:36 | UserToken |
26+
| JsonWebToken.js:48:11:48:28 | UserToken |
27+
| JsonWebToken.js:48:23:48:28 | aJwt() |
28+
| JsonWebToken.js:48:23:48:28 | aJwt() |
29+
| JsonWebToken.js:51:28:51:36 | UserToken |
30+
| JsonWebToken.js:51:28:51:36 | UserToken |
31+
| JsonWebToken.js:52:28:52:36 | UserToken |
32+
| JsonWebToken.js:52:28:52:36 | UserToken |
33+
| jose.js:18:11:18:28 | UserToken |
34+
| jose.js:18:23:18:28 | aJwt() |
35+
| jose.js:18:23:18:28 | aJwt() |
36+
| jose.js:21:20:21:28 | UserToken |
37+
| jose.js:21:20:21:28 | UserToken |
38+
| jose.js:25:11:25:28 | UserToken |
39+
| jose.js:25:23:25:28 | aJwt() |
40+
| jose.js:25:23:25:28 | aJwt() |
41+
| jose.js:28:20:28:28 | UserToken |
42+
| jose.js:28:20:28:28 | UserToken |
43+
| jose.js:29:26:29:34 | UserToken |
44+
| jose.js:29:26:29:34 | UserToken |
45+
| jose.js:33:11:33:28 | UserToken |
46+
| jose.js:33:23:33:28 | aJwt() |
47+
| jose.js:33:23:33:28 | aJwt() |
48+
| jose.js:36:26:36:34 | UserToken |
49+
| jose.js:36:26:36:34 | UserToken |
50+
| jwtDecode.js:18:11:18:28 | UserToken |
51+
| jwtDecode.js:18:23:18:28 | aJwt() |
52+
| jwtDecode.js:18:23:18:28 | aJwt() |
53+
| jwtDecode.js:22:16:22:24 | UserToken |
54+
| jwtDecode.js:22:16:22:24 | UserToken |
55+
| jwtSimple.js:18:11:18:28 | UserToken |
56+
| jwtSimple.js:18:23:18:28 | aJwt() |
57+
| jwtSimple.js:18:23:18:28 | aJwt() |
58+
| jwtSimple.js:21:23:21:31 | UserToken |
59+
| jwtSimple.js:21:23:21:31 | UserToken |
60+
| jwtSimple.js:25:11:25:28 | UserToken |
61+
| jwtSimple.js:25:23:25:28 | aJwt() |
62+
| jwtSimple.js:25:23:25:28 | aJwt() |
63+
| jwtSimple.js:28:23:28:31 | UserToken |
64+
| jwtSimple.js:28:23:28:31 | UserToken |
65+
| jwtSimple.js:29:23:29:31 | UserToken |
66+
| jwtSimple.js:29:23:29:31 | UserToken |
67+
| jwtSimple.js:33:11:33:28 | UserToken |
68+
| jwtSimple.js:33:23:33:28 | aJwt() |
69+
| jwtSimple.js:33:23:33:28 | aJwt() |
70+
| jwtSimple.js:36:23:36:31 | UserToken |
71+
| jwtSimple.js:36:23:36:31 | UserToken |
72+
| jwtSimple.js:37:23:37:31 | UserToken |
73+
| jwtSimple.js:37:23:37:31 | UserToken |
74+
edges
75+
| JsonWebToken.js:18:11:18:28 | UserToken | JsonWebToken.js:21:28:21:36 | UserToken |
76+
| JsonWebToken.js:18:11:18:28 | UserToken | JsonWebToken.js:21:28:21:36 | UserToken |
77+
| JsonWebToken.js:18:23:18:28 | aJwt() | JsonWebToken.js:18:11:18:28 | UserToken |
78+
| JsonWebToken.js:18:23:18:28 | aJwt() | JsonWebToken.js:18:11:18:28 | UserToken |
79+
| JsonWebToken.js:25:11:25:28 | UserToken | JsonWebToken.js:28:28:28:36 | UserToken |
80+
| JsonWebToken.js:25:11:25:28 | UserToken | JsonWebToken.js:28:28:28:36 | UserToken |
81+
| JsonWebToken.js:25:11:25:28 | UserToken | JsonWebToken.js:29:28:29:36 | UserToken |
82+
| JsonWebToken.js:25:11:25:28 | UserToken | JsonWebToken.js:29:28:29:36 | UserToken |
83+
| JsonWebToken.js:25:23:25:28 | aJwt() | JsonWebToken.js:25:11:25:28 | UserToken |
84+
| JsonWebToken.js:25:23:25:28 | aJwt() | JsonWebToken.js:25:11:25:28 | UserToken |
85+
| JsonWebToken.js:33:11:33:28 | UserToken | JsonWebToken.js:36:28:36:36 | UserToken |
86+
| JsonWebToken.js:33:11:33:28 | UserToken | JsonWebToken.js:36:28:36:36 | UserToken |
87+
| JsonWebToken.js:33:23:33:28 | aJwt() | JsonWebToken.js:33:11:33:28 | UserToken |
88+
| JsonWebToken.js:33:23:33:28 | aJwt() | JsonWebToken.js:33:11:33:28 | UserToken |
89+
| JsonWebToken.js:40:11:40:28 | UserToken | JsonWebToken.js:43:28:43:36 | UserToken |
90+
| JsonWebToken.js:40:11:40:28 | UserToken | JsonWebToken.js:43:28:43:36 | UserToken |
91+
| JsonWebToken.js:40:11:40:28 | UserToken | JsonWebToken.js:44:28:44:36 | UserToken |
92+
| JsonWebToken.js:40:11:40:28 | UserToken | JsonWebToken.js:44:28:44:36 | UserToken |
93+
| JsonWebToken.js:40:23:40:28 | aJwt() | JsonWebToken.js:40:11:40:28 | UserToken |
94+
| JsonWebToken.js:40:23:40:28 | aJwt() | JsonWebToken.js:40:11:40:28 | UserToken |
95+
| JsonWebToken.js:48:11:48:28 | UserToken | JsonWebToken.js:51:28:51:36 | UserToken |
96+
| JsonWebToken.js:48:11:48:28 | UserToken | JsonWebToken.js:51:28:51:36 | UserToken |
97+
| JsonWebToken.js:48:11:48:28 | UserToken | JsonWebToken.js:52:28:52:36 | UserToken |
98+
| JsonWebToken.js:48:11:48:28 | UserToken | JsonWebToken.js:52:28:52:36 | UserToken |
99+
| JsonWebToken.js:48:23:48:28 | aJwt() | JsonWebToken.js:48:11:48:28 | UserToken |
100+
| JsonWebToken.js:48:23:48:28 | aJwt() | JsonWebToken.js:48:11:48:28 | UserToken |
101+
| jose.js:18:11:18:28 | UserToken | jose.js:21:20:21:28 | UserToken |
102+
| jose.js:18:11:18:28 | UserToken | jose.js:21:20:21:28 | UserToken |
103+
| jose.js:18:23:18:28 | aJwt() | jose.js:18:11:18:28 | UserToken |
104+
| jose.js:18:23:18:28 | aJwt() | jose.js:18:11:18:28 | UserToken |
105+
| jose.js:25:11:25:28 | UserToken | jose.js:28:20:28:28 | UserToken |
106+
| jose.js:25:11:25:28 | UserToken | jose.js:28:20:28:28 | UserToken |
107+
| jose.js:25:11:25:28 | UserToken | jose.js:29:26:29:34 | UserToken |
108+
| jose.js:25:11:25:28 | UserToken | jose.js:29:26:29:34 | UserToken |
109+
| jose.js:25:23:25:28 | aJwt() | jose.js:25:11:25:28 | UserToken |
110+
| jose.js:25:23:25:28 | aJwt() | jose.js:25:11:25:28 | UserToken |
111+
| jose.js:33:11:33:28 | UserToken | jose.js:36:26:36:34 | UserToken |
112+
| jose.js:33:11:33:28 | UserToken | jose.js:36:26:36:34 | UserToken |
113+
| jose.js:33:23:33:28 | aJwt() | jose.js:33:11:33:28 | UserToken |
114+
| jose.js:33:23:33:28 | aJwt() | jose.js:33:11:33:28 | UserToken |
115+
| jwtDecode.js:18:11:18:28 | UserToken | jwtDecode.js:22:16:22:24 | UserToken |
116+
| jwtDecode.js:18:11:18:28 | UserToken | jwtDecode.js:22:16:22:24 | UserToken |
117+
| jwtDecode.js:18:23:18:28 | aJwt() | jwtDecode.js:18:11:18:28 | UserToken |
118+
| jwtDecode.js:18:23:18:28 | aJwt() | jwtDecode.js:18:11:18:28 | UserToken |
119+
| jwtSimple.js:18:11:18:28 | UserToken | jwtSimple.js:21:23:21:31 | UserToken |
120+
| jwtSimple.js:18:11:18:28 | UserToken | jwtSimple.js:21:23:21:31 | UserToken |
121+
| jwtSimple.js:18:23:18:28 | aJwt() | jwtSimple.js:18:11:18:28 | UserToken |
122+
| jwtSimple.js:18:23:18:28 | aJwt() | jwtSimple.js:18:11:18:28 | UserToken |
123+
| jwtSimple.js:25:11:25:28 | UserToken | jwtSimple.js:28:23:28:31 | UserToken |
124+
| jwtSimple.js:25:11:25:28 | UserToken | jwtSimple.js:28:23:28:31 | UserToken |
125+
| jwtSimple.js:25:11:25:28 | UserToken | jwtSimple.js:29:23:29:31 | UserToken |
126+
| jwtSimple.js:25:11:25:28 | UserToken | jwtSimple.js:29:23:29:31 | UserToken |
127+
| jwtSimple.js:25:23:25:28 | aJwt() | jwtSimple.js:25:11:25:28 | UserToken |
128+
| jwtSimple.js:25:23:25:28 | aJwt() | jwtSimple.js:25:11:25:28 | UserToken |
129+
| jwtSimple.js:33:11:33:28 | UserToken | jwtSimple.js:36:23:36:31 | UserToken |
130+
| jwtSimple.js:33:11:33:28 | UserToken | jwtSimple.js:36:23:36:31 | UserToken |
131+
| jwtSimple.js:33:11:33:28 | UserToken | jwtSimple.js:37:23:37:31 | UserToken |
132+
| jwtSimple.js:33:11:33:28 | UserToken | jwtSimple.js:37:23:37:31 | UserToken |
133+
| jwtSimple.js:33:23:33:28 | aJwt() | jwtSimple.js:33:11:33:28 | UserToken |
134+
| jwtSimple.js:33:23:33:28 | aJwt() | jwtSimple.js:33:11:33:28 | UserToken |
135+
#select
136+
| JsonWebToken.js:18:23:18:28 | aJwt() | JsonWebToken.js:18:23:18:28 | aJwt() | JsonWebToken.js:21:28:21:36 | UserToken | Decoding JWT $@. | JsonWebToken.js:21:28:21:36 | UserToken | without signature verification |
137+
| JsonWebToken.js:25:23:25:28 | aJwt() | JsonWebToken.js:25:23:25:28 | aJwt() | JsonWebToken.js:28:28:28:36 | UserToken | Decoding JWT $@. | JsonWebToken.js:28:28:28:36 | UserToken | without signature verification |
138+
| JsonWebToken.js:25:23:25:28 | aJwt() | JsonWebToken.js:25:23:25:28 | aJwt() | JsonWebToken.js:29:28:29:36 | UserToken | Decoding JWT $@. | JsonWebToken.js:29:28:29:36 | UserToken | without signature verification |
139+
| jose.js:18:23:18:28 | aJwt() | jose.js:18:23:18:28 | aJwt() | jose.js:21:20:21:28 | UserToken | Decoding JWT $@. | jose.js:21:20:21:28 | UserToken | without signature verification |
140+
| jwtDecode.js:18:23:18:28 | aJwt() | jwtDecode.js:18:23:18:28 | aJwt() | jwtDecode.js:22:16:22:24 | UserToken | Decoding JWT $@. | jwtDecode.js:22:16:22:24 | UserToken | without signature verification |
141+
| jwtSimple.js:18:23:18:28 | aJwt() | jwtSimple.js:18:23:18:28 | aJwt() | jwtSimple.js:21:23:21:31 | UserToken | Decoding JWT $@. | jwtSimple.js:21:23:21:31 | UserToken | without signature verification |

‎javascript/ql/test/experimental/Security/CWE-347/decodeJwtWithoutVerificationLocalSource.qlref renamed to ‎javascript/ql/test/experimental/Security/CWE-347/localsource/decodeJwtWithoutVerificationLocalSource.qlref

File renamed without changes.

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
const express = require('express')
2+
const app = express()
3+
const jwtJsonwebtoken = require('jsonwebtoken');
4+
const jwt_decode = require('jwt-decode');
5+
const jwt_simple = require('jwt-simple');
6+
const jose = require('jose')
7+
const port = 3000
8+
9+
function getSecret() {
10+
return "A Safe generated random key"
11+
}
12+
13+
function aJWT() {
14+
return "A JWT provided by user"
15+
}
16+
17+
(function () {
18+
const UserToken = aJwt()
19+
20+
// no signature verification
21+
jose.decodeJwt(UserToken) // NOT OK
22+
})();
23+
24+
(async function () {
25+
const UserToken = aJwt()
26+
27+
// first without signature verification then with signature verification for same UserToken
28+
jose.decodeJwt(UserToken) // OK
29+
await jose.jwtVerify(UserToken, new TextEncoder().encode(getSecret())) // OK
30+
})();
31+
32+
(async function () {
33+
const UserToken = aJwt()
34+
35+
// with signature verification
36+
await jose.jwtVerify(UserToken, new TextEncoder().encode(getSecret())) // OK
37+
})();

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
const express = require('express')
2+
const app = express()
3+
const jwtJsonwebtoken = require('jsonwebtoken');
4+
const jwt_decode = require('jwt-decode');
5+
const jwt_simple = require('jwt-simple');
6+
const jose = require('jose')
7+
const port = 3000
8+
9+
function getSecret() {
10+
return "A Safe generated random key"
11+
}
12+
13+
function aJWT() {
14+
return "A JWT provided by user"
15+
}
16+
17+
(function () {
18+
const UserToken = aJwt()
19+
20+
// jwt-decode
21+
// no signature verification
22+
jwt_decode(UserToken) // NOT OK
23+
})();

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
const express = require('express')
2+
const app = express()
3+
const jwtJsonwebtoken = require('jsonwebtoken');
4+
const jwt_decode = require('jwt-decode');
5+
const jwt_simple = require('jwt-simple');
6+
const jose = require('jose')
7+
const port = 3000
8+
9+
function getSecret() {
10+
return "A Safe generated random key"
11+
}
12+
13+
function aJWT() {
14+
return "A JWT provided by user"
15+
}
16+
17+
(function () {
18+
const UserToken = aJwt()
19+
20+
// BAD: no signature verification
21+
jwt_simple.decode(UserToken, getSecret(), true); // NOT OK
22+
})();
23+
24+
(function () {
25+
const UserToken = aJwt()
26+
27+
// GOOD: all with with signature verification
28+
jwt_simple.decode(UserToken, getSecret(), false); // OK
29+
jwt_simple.decode(UserToken, getSecret()); // OK
30+
})();
31+
32+
(function () {
33+
const UserToken = aJwt()
34+
35+
// GOOD: first without signature verification then with signature verification for same UserToken
36+
jwt_simple.decode(UserToken, getSecret(), true); // OK
37+
jwt_simple.decode(UserToken, getSecret()); // OK
38+
})();

0 commit comments

Comments
 (0)