Skip to content

Commit 13edc37

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#4638 from erik-krogh/jwt
Approved by asgerf
2 parents 8bb9e8a + 7cf7a44 commit 13edc37

File tree

9 files changed

+144
-1
lines changed

9 files changed

+144
-1
lines changed

‎javascript/change-notes/2020-11-09-jwt.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
lgtm,codescanning
2+
* The security queries now track taint through JWT decoding, and warns about hard-coded JWT signing keys.
3+
Affected packages are
4+
[jsonwebtoken](https://www.npmjs.com/package/jsonwebtoken) and
5+
[jwt-decode](https://www.npmjs.com/package/jwt-decode)

‎javascript/ql/src/javascript.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ import semmle.javascript.frameworks.EventEmitter
8686
import semmle.javascript.frameworks.Files
8787
import semmle.javascript.frameworks.Firebase
8888
import semmle.javascript.frameworks.jQuery
89+
import semmle.javascript.frameworks.JWT
8990
import semmle.javascript.frameworks.Handlebars
9091
import semmle.javascript.frameworks.LazyCache
9192
import semmle.javascript.frameworks.LodashUnderscore

‎javascript/ql/src/semmle/javascript/frameworks/JWT.qll

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* Provides classes for working with JWT libraries.
3+
*/
4+
5+
import javascript
6+
7+
/**
8+
* Provides classes and predicates modeling the `jwt-decode` libary.
9+
*/
10+
private module JwtDecode {
11+
/**
12+
* A taint-step for `succ = require("jwt-decode")(pred)`.
13+
*/
14+
private class JwtDecodeStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
15+
JwtDecodeStep() { this = DataFlow::moduleImport("jwt-decode").getACall() }
16+
17+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
18+
pred = this.getArgument(0) and
19+
succ = this
20+
}
21+
}
22+
}
23+
24+
/**
25+
* Provides classes and predicates modeling the `jsonwebtoken` libary.
26+
*/
27+
private module JsonWebToken {
28+
/**
29+
* A taint-step for `require("jsonwebtoken").verify(pred, "key", (err succ) => {...})`.
30+
*/
31+
private class VerifyStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
32+
VerifyStep() { this = DataFlow::moduleMember("jsonwebtoken", "verify").getACall() }
33+
34+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
35+
pred = this.getArgument(0) and
36+
succ = this.getABoundCallbackParameter(2, 1)
37+
}
38+
}
39+
40+
/**
41+
* The private key for a JWT as a `CredentialsExpr`.
42+
*/
43+
private class JWTKey extends CredentialsExpr {
44+
JWTKey() {
45+
this = DataFlow::moduleMember("jsonwebtoken", "sign").getACall().getArgument(1).asExpr()
46+
}
47+
48+
override string getCredentialsKind() { result = "key" }
49+
}
50+
}

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,14 @@ nodes
151151
| jquery.js:16:38:16:52 | window.___location |
152152
| jquery.js:16:38:16:52 | window.___location |
153153
| jquery.js:16:38:16:63 | window. ... tring() |
154+
| jwt-server.js:7:9:7:35 | taint |
155+
| jwt-server.js:7:17:7:35 | req.param("wobble") |
156+
| jwt-server.js:7:17:7:35 | req.param("wobble") |
157+
| jwt-server.js:9:16:9:20 | taint |
158+
| jwt-server.js:9:55:9:61 | decoded |
159+
| jwt-server.js:11:19:11:25 | decoded |
160+
| jwt-server.js:11:19:11:29 | decoded.foo |
161+
| jwt-server.js:11:19:11:29 | decoded.foo |
154162
| nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` |
155163
| nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` |
156164
| nodemailer.js:13:50:13:66 | req.query.message |
@@ -767,6 +775,13 @@ edges
767775
| jquery.js:16:38:16:52 | window.___location | jquery.js:16:38:16:63 | window. ... tring() |
768776
| jquery.js:16:38:16:63 | window. ... tring() | jquery.js:16:19:16:64 | decodeU ... ring()) |
769777
| jquery.js:16:38:16:63 | window. ... tring() | jquery.js:16:19:16:64 | decodeU ... ring()) |
778+
| jwt-server.js:7:9:7:35 | taint | jwt-server.js:9:16:9:20 | taint |
779+
| jwt-server.js:7:17:7:35 | req.param("wobble") | jwt-server.js:7:9:7:35 | taint |
780+
| jwt-server.js:7:17:7:35 | req.param("wobble") | jwt-server.js:7:9:7:35 | taint |
781+
| jwt-server.js:9:16:9:20 | taint | jwt-server.js:9:55:9:61 | decoded |
782+
| jwt-server.js:9:55:9:61 | decoded | jwt-server.js:11:19:11:25 | decoded |
783+
| jwt-server.js:11:19:11:25 | decoded | jwt-server.js:11:19:11:29 | decoded.foo |
784+
| jwt-server.js:11:19:11:25 | decoded | jwt-server.js:11:19:11:29 | decoded.foo |
770785
| nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` |
771786
| nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` |
772787
| nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` |
@@ -1242,6 +1257,7 @@ edges
12421257
| jquery.js:14:19:14:58 | decodeU ... n.hash) | jquery.js:14:38:14:52 | window.___location | jquery.js:14:19:14:58 | decodeU ... n.hash) | Cross-site scripting vulnerability due to $@. | jquery.js:14:38:14:52 | window.___location | user-provided value |
12431258
| jquery.js:15:19:15:60 | decodeU ... search) | jquery.js:15:38:15:52 | window.___location | jquery.js:15:19:15:60 | decodeU ... search) | Cross-site scripting vulnerability due to $@. | jquery.js:15:38:15:52 | window.___location | user-provided value |
12441259
| jquery.js:16:19:16:64 | decodeU ... ring()) | jquery.js:16:38:16:52 | window.___location | jquery.js:16:19:16:64 | decodeU ... ring()) | Cross-site scripting vulnerability due to $@. | jquery.js:16:38:16:52 | window.___location | user-provided value |
1260+
| jwt-server.js:11:19:11:29 | decoded.foo | jwt-server.js:7:17:7:35 | req.param("wobble") | jwt-server.js:11:19:11:29 | decoded.foo | Cross-site scripting vulnerability due to $@. | jwt-server.js:7:17:7:35 | req.param("wobble") | user-provided value |
12451261
| nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | HTML injection vulnerability due to $@. | nodemailer.js:13:50:13:66 | req.query.message | user-provided value |
12461262
| optionalSanitizer.js:6:18:6:23 | target | optionalSanitizer.js:2:16:2:32 | document.___location | optionalSanitizer.js:6:18:6:23 | target | Cross-site scripting vulnerability due to $@. | optionalSanitizer.js:2:16:2:32 | document.___location | user-provided value |
12471263
| optionalSanitizer.js:9:18:9:24 | tainted | optionalSanitizer.js:2:16:2:32 | document.___location | optionalSanitizer.js:9:18:9:24 | tainted | Cross-site scripting vulnerability due to $@. | optionalSanitizer.js:2:16:2:32 | document.___location | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,21 @@ nodes
151151
| jquery.js:16:38:16:52 | window.___location |
152152
| jquery.js:16:38:16:52 | window.___location |
153153
| jquery.js:16:38:16:63 | window. ... tring() |
154+
| jwt-server.js:7:9:7:35 | taint |
155+
| jwt-server.js:7:17:7:35 | req.param("wobble") |
156+
| jwt-server.js:7:17:7:35 | req.param("wobble") |
157+
| jwt-server.js:9:16:9:20 | taint |
158+
| jwt-server.js:9:55:9:61 | decoded |
159+
| jwt-server.js:11:19:11:25 | decoded |
160+
| jwt-server.js:11:19:11:29 | decoded.foo |
161+
| jwt-server.js:11:19:11:29 | decoded.foo |
162+
| jwt.js:4:36:4:39 | data |
163+
| jwt.js:4:36:4:39 | data |
164+
| jwt.js:5:9:5:34 | decoded |
165+
| jwt.js:5:19:5:34 | jwt_decode(data) |
166+
| jwt.js:5:30:5:33 | data |
167+
| jwt.js:6:14:6:20 | decoded |
168+
| jwt.js:6:14:6:20 | decoded |
154169
| nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` |
155170
| nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` |
156171
| nodemailer.js:13:50:13:66 | req.query.message |
@@ -771,6 +786,19 @@ edges
771786
| jquery.js:16:38:16:52 | window.___location | jquery.js:16:38:16:63 | window. ... tring() |
772787
| jquery.js:16:38:16:63 | window. ... tring() | jquery.js:16:19:16:64 | decodeU ... ring()) |
773788
| jquery.js:16:38:16:63 | window. ... tring() | jquery.js:16:19:16:64 | decodeU ... ring()) |
789+
| jwt-server.js:7:9:7:35 | taint | jwt-server.js:9:16:9:20 | taint |
790+
| jwt-server.js:7:17:7:35 | req.param("wobble") | jwt-server.js:7:9:7:35 | taint |
791+
| jwt-server.js:7:17:7:35 | req.param("wobble") | jwt-server.js:7:9:7:35 | taint |
792+
| jwt-server.js:9:16:9:20 | taint | jwt-server.js:9:55:9:61 | decoded |
793+
| jwt-server.js:9:55:9:61 | decoded | jwt-server.js:11:19:11:25 | decoded |
794+
| jwt-server.js:11:19:11:25 | decoded | jwt-server.js:11:19:11:29 | decoded.foo |
795+
| jwt-server.js:11:19:11:25 | decoded | jwt-server.js:11:19:11:29 | decoded.foo |
796+
| jwt.js:4:36:4:39 | data | jwt.js:5:30:5:33 | data |
797+
| jwt.js:4:36:4:39 | data | jwt.js:5:30:5:33 | data |
798+
| jwt.js:5:9:5:34 | decoded | jwt.js:6:14:6:20 | decoded |
799+
| jwt.js:5:9:5:34 | decoded | jwt.js:6:14:6:20 | decoded |
800+
| jwt.js:5:19:5:34 | jwt_decode(data) | jwt.js:5:9:5:34 | decoded |
801+
| jwt.js:5:30:5:33 | data | jwt.js:5:19:5:34 | jwt_decode(data) |
774802
| nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` |
775803
| nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` |
776804
| nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` |
@@ -1216,4 +1244,5 @@ edges
12161244
| winjs.js:2:17:2:40 | documen ... .search | winjs.js:2:17:2:53 | documen ... ring(1) |
12171245
| winjs.js:2:17:2:53 | documen ... ring(1) | winjs.js:2:7:2:53 | tainted |
12181246
#select
1247+
| jwt.js:6:14:6:20 | decoded | jwt.js:4:36:4:39 | data | jwt.js:6:14:6:20 | decoded | Cross-site scripting vulnerability due to $@. | jwt.js:4:36:4:39 | data | user-provided value |
12191248
| typeahead.js:10:16:10:18 | loc | typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc | Cross-site scripting vulnerability due to $@. | typeahead.js:9:28:9:30 | loc | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/jwt-server.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
var express = require('express');
2+
var app = express();
3+
import jwt from "jsonwebtoken";
4+
5+
import { JSDOM } from "jsdom";
6+
app.get('/some/path', function (req, res) {
7+
var taint = req.param("wobble");
8+
9+
jwt.verify(taint, 'my-secret-key', function (err, decoded) {
10+
// NOT OK
11+
new JSDOM(decoded.foo, { runScripts: "dangerously" });
12+
});
13+
});

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/jwt.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import jwt_decode from "jwt-decode";
2+
import $ from "jquery"
3+
4+
$.post(loginUrl(), {data: "foo"}, (data, xhr) => {
5+
var decoded = jwt_decode(data);
6+
$.jGrowl(decoded); // NOT OK - but only flagged with additional sources [INCONSISTENCY]
7+
});

‎javascript/ql/test/query-tests/Security/CWE-798/HardcodedCredentials.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,11 @@ nodes
208208
| HardcodedCredentials.js:237:35:237:91 | Buffer. ... ase64') |
209209
| HardcodedCredentials.js:237:47:237:54 | username |
210210
| HardcodedCredentials.js:237:47:237:71 | usernam ... assword |
211+
| HardcodedCredentials.js:245:9:245:44 | privateKey |
212+
| HardcodedCredentials.js:245:22:245:44 | "myHard ... ateKey" |
213+
| HardcodedCredentials.js:245:22:245:44 | "myHard ... ateKey" |
214+
| HardcodedCredentials.js:246:42:246:51 | privateKey |
215+
| HardcodedCredentials.js:246:42:246:51 | privateKey |
211216
edges
212217
| HardcodedCredentials.js:5:15:5:22 | 'dbuser' | HardcodedCredentials.js:5:15:5:22 | 'dbuser' |
213218
| HardcodedCredentials.js:8:19:8:28 | 'abcdefgh' | HardcodedCredentials.js:8:19:8:28 | 'abcdefgh' |
@@ -309,6 +314,10 @@ edges
309314
| HardcodedCredentials.js:237:35:237:91 | Buffer. ... ase64') | HardcodedCredentials.js:237:24:237:91 | 'Basic ... ase64') |
310315
| HardcodedCredentials.js:237:47:237:54 | username | HardcodedCredentials.js:237:47:237:71 | usernam ... assword |
311316
| HardcodedCredentials.js:237:47:237:71 | usernam ... assword | HardcodedCredentials.js:237:35:237:72 | Buffer. ... ssword) |
317+
| HardcodedCredentials.js:245:9:245:44 | privateKey | HardcodedCredentials.js:246:42:246:51 | privateKey |
318+
| HardcodedCredentials.js:245:9:245:44 | privateKey | HardcodedCredentials.js:246:42:246:51 | privateKey |
319+
| HardcodedCredentials.js:245:22:245:44 | "myHard ... ateKey" | HardcodedCredentials.js:245:9:245:44 | privateKey |
320+
| HardcodedCredentials.js:245:22:245:44 | "myHard ... ateKey" | HardcodedCredentials.js:245:9:245:44 | privateKey |
312321
#select
313322
| HardcodedCredentials.js:5:15:5:22 | 'dbuser' | HardcodedCredentials.js:5:15:5:22 | 'dbuser' | HardcodedCredentials.js:5:15:5:22 | 'dbuser' | The hard-coded value "dbuser" is used as $@. | HardcodedCredentials.js:5:15:5:22 | 'dbuser' | user name |
314323
| HardcodedCredentials.js:8:19:8:28 | 'abcdefgh' | HardcodedCredentials.js:8:19:8:28 | 'abcdefgh' | HardcodedCredentials.js:8:19:8:28 | 'abcdefgh' | The hard-coded value "abcdefgh" is used as $@. | HardcodedCredentials.js:8:19:8:28 | 'abcdefgh' | password |
@@ -374,3 +383,4 @@ edges
374383
| HardcodedCredentials.js:214:18:214:25 | 'sdsdag' | HardcodedCredentials.js:214:18:214:25 | 'sdsdag' | HardcodedCredentials.js:221:37:221:51 | `Basic ${AUTH}` | The hard-coded value "sdsdag" is used as $@. | HardcodedCredentials.js:221:37:221:51 | `Basic ${AUTH}` | authorization header |
375384
| HardcodedCredentials.js:215:18:215:25 | 'sdsdag' | HardcodedCredentials.js:215:18:215:25 | 'sdsdag' | HardcodedCredentials.js:221:37:221:51 | `Basic ${AUTH}` | The hard-coded value "sdsdag" is used as $@. | HardcodedCredentials.js:221:37:221:51 | `Basic ${AUTH}` | authorization header |
376385
| HardcodedCredentials.js:231:22:231:29 | 'sdsdag' | HardcodedCredentials.js:231:22:231:29 | 'sdsdag' | HardcodedCredentials.js:237:24:237:91 | 'Basic ... ase64') | The hard-coded value "sdsdag" is used as $@. | HardcodedCredentials.js:237:24:237:91 | 'Basic ... ase64') | authorization header |
386+
| HardcodedCredentials.js:245:22:245:44 | "myHard ... ateKey" | HardcodedCredentials.js:245:22:245:44 | "myHard ... ateKey" | HardcodedCredentials.js:246:42:246:51 | privateKey | The hard-coded value "myHardCodedPrivateKey" is used as $@. | HardcodedCredentials.js:246:42:246:51 | privateKey | key |

‎javascript/ql/test/query-tests/Security/CWE-798/HardcodedCredentials.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,4 +237,16 @@
237237
Authorization: 'Basic ' + Buffer.from(username + ':' + password).toString('base64'),
238238
},
239239
});
240-
})
240+
})
241+
242+
(function () {
243+
import jwt from "jsonwebtoken";
244+
245+
var privateKey = "myHardCodedPrivateKey";
246+
var token = jwt.sign({ foo: 'bar' }, privateKey, { algorithm: 'RS256'});
247+
248+
var publicKey = "myHardCodedPublicKey";
249+
jwt.verify(token, publicKey, function(err, decoded) {
250+
console.log(decoded);
251+
});
252+
})();

0 commit comments

Comments
 (0)