Skip to content

Commit 23c3be6

Browse files
erik-krogherik-krogh
committed
add support for the json-cycle library
1 parent 94cbc4b commit 23c3be6

File tree

9 files changed

+47
-4
lines changed

9 files changed

+47
-4
lines changed

‎javascript/change-notes/2021-06-24-json.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@ lgtm,codescanning
88
[teleport-javascript](https://npmjs.com/package/teleport-javascript),
99
[replicator](https://npmjs.com/package/replicator),
1010
[safe-stable-stringify](https://npmjs.com/package/safe-stable-stringify),
11-
[fclone](https://npmjs.com/package/fclone)
11+
[fclone](https://npmjs.com/package/fclone),
12+
[json-cycle](https://npmjs.com/package/json-cycle)

‎javascript/ql/src/semmle/javascript/Extend.qll‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,11 @@ private import semmle.javascript.dataflow.internal.PreCallGraphStep
182182
*/
183183
private class CloneStep extends PreCallGraphStep {
184184
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
185-
exists(DataFlow::CallNode call | call = DataFlow::moduleImport(["clone", "fclone"]).getACall() |
185+
exists(DataFlow::CallNode call |
186+
call = DataFlow::moduleImport(["clone", "fclone"]).getACall()
187+
or
188+
call = DataFlow::moduleMember("json-cycle", ["decycle", "retrocycle"]).getACall()
189+
|
186190
pred = call.getArgument(0) and
187191
succ = call
188192
)

‎javascript/ql/src/semmle/javascript/JsonParsers.qll‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ private class PlainJsonParserCall extends JsonParserCall {
2626
PlainJsonParserCall() {
2727
exists(DataFlow::SourceNode callee | this = callee.getACall() |
2828
callee = DataFlow::globalVarRef("JSON").getAPropertyRead("parse") or
29-
callee = DataFlow::moduleMember(["json3", "json5", "flatted", "teleport-javascript"], "parse") or
29+
callee =
30+
DataFlow::moduleMember(["json3", "json5", "flatted", "teleport-javascript", "json-cycle"],
31+
"parse") or
3032
callee = API::moduleImport("replicator").getInstance().getMember("decode").getAnImmediateUse() or
3133
callee = DataFlow::moduleImport("parse-json") or
3234
callee = DataFlow::moduleImport("json-parse-better-errors") or

‎javascript/ql/src/semmle/javascript/JsonStringifiers.qll‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ class JsonStringifyCall extends DataFlow::CallNode {
1212
exists(DataFlow::SourceNode callee | this = callee.getACall() |
1313
callee = DataFlow::globalVarRef("JSON").getAPropertyRead("stringify") or
1414
callee =
15-
DataFlow::moduleMember(["json3", "json5", "flatted", "teleport-javascript"], "stringify") or
15+
DataFlow::moduleMember(["json3", "json5", "flatted", "teleport-javascript", "json-cycle"],
16+
"stringify") or
1617
callee = API::moduleImport("replicator").getInstance().getMember("encode").getAnImmediateUse() or
1718
callee =
1819
DataFlow::moduleImport([

‎javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ typeInferenceMismatch
9696
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:30:8:30:49 | telepor ... ource)) |
9797
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:34:8:34:51 | replica ... ource)) |
9898
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:36:8:36:47 | require ... source) |
99+
| json-stringify.js:2:16:2:23 | source() | json-stringify.js:39:8:39:37 | jc.stri ... ource)) |
99100
| json-stringify.js:3:15:3:22 | source() | json-stringify.js:8:8:8:31 | jsonStr ... (taint) |
100101
| nested-props.js:4:13:4:20 | source() | nested-props.js:5:10:5:14 | obj.x |
101102
| nested-props.js:9:18:9:25 | source() | nested-props.js:10:10:10:16 | obj.x.y |

‎javascript/ql/test/library-tests/TaintTracking/json-stringify.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,7 @@ function foo() {
3434
sink(replicator.encode(replicator.decode(source))); // NOT OK
3535

3636
sink(require("safe-stable-stringify")(source)); // NOT OK
37+
38+
const jc = require('json-cycle');
39+
sink(jc.stringify(jc.parse(source))); // NOT OK
3740
}

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,14 @@ nodes
198198
| tst2.js:63:12:63:12 | p |
199199
| tst2.js:64:12:64:18 | other.p |
200200
| tst2.js:64:12:64:18 | other.p |
201+
| tst2.js:69:7:69:24 | p |
202+
| tst2.js:69:9:69:9 | p |
203+
| tst2.js:69:9:69:9 | p |
204+
| tst2.js:72:11:72:11 | p |
205+
| tst2.js:75:12:75:12 | p |
206+
| tst2.js:75:12:75:12 | p |
207+
| tst2.js:76:12:76:18 | other.p |
208+
| tst2.js:76:12:76:18 | other.p |
201209
| tst3.js:5:7:5:24 | p |
202210
| tst3.js:5:9:5:9 | p |
203211
| tst3.js:5:9:5:9 | p |
@@ -374,6 +382,13 @@ edges
374382
| tst2.js:57:9:57:9 | p | tst2.js:57:7:57:24 | p |
375383
| tst2.js:60:11:60:11 | p | tst2.js:64:12:64:18 | other.p |
376384
| tst2.js:60:11:60:11 | p | tst2.js:64:12:64:18 | other.p |
385+
| tst2.js:69:7:69:24 | p | tst2.js:72:11:72:11 | p |
386+
| tst2.js:69:7:69:24 | p | tst2.js:75:12:75:12 | p |
387+
| tst2.js:69:7:69:24 | p | tst2.js:75:12:75:12 | p |
388+
| tst2.js:69:9:69:9 | p | tst2.js:69:7:69:24 | p |
389+
| tst2.js:69:9:69:9 | p | tst2.js:69:7:69:24 | p |
390+
| tst2.js:72:11:72:11 | p | tst2.js:76:12:76:18 | other.p |
391+
| tst2.js:72:11:72:11 | p | tst2.js:76:12:76:18 | other.p |
377392
| tst3.js:5:7:5:24 | p | tst3.js:6:12:6:12 | p |
378393
| tst3.js:5:7:5:24 | p | tst3.js:6:12:6:12 | p |
379394
| tst3.js:5:9:5:9 | p | tst3.js:5:7:5:24 | p |
@@ -429,5 +444,7 @@ edges
429444
| tst2.js:51:12:51:17 | unsafe | tst2.js:43:9:43:9 | p | tst2.js:51:12:51:17 | unsafe | Cross-site scripting vulnerability due to $@. | tst2.js:43:9:43:9 | p | user-provided value |
430445
| tst2.js:63:12:63:12 | p | tst2.js:57:9:57:9 | p | tst2.js:63:12:63:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:57:9:57:9 | p | user-provided value |
431446
| tst2.js:64:12:64:18 | other.p | tst2.js:57:9:57:9 | p | tst2.js:64:12:64:18 | other.p | Cross-site scripting vulnerability due to $@. | tst2.js:57:9:57:9 | p | user-provided value |
447+
| tst2.js:75:12:75:12 | p | tst2.js:69:9:69:9 | p | tst2.js:75:12:75:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:69:9:69:9 | p | user-provided value |
448+
| tst2.js:76:12:76:18 | other.p | tst2.js:69:9:69:9 | p | tst2.js:76:12:76:18 | other.p | Cross-site scripting vulnerability due to $@. | tst2.js:69:9:69:9 | p | user-provided value |
432449
| tst3.js:6:12:6:12 | p | tst3.js:5:9:5:9 | p | tst3.js:6:12:6:12 | p | Cross-site scripting vulnerability due to $@. | tst3.js:5:9:5:9 | p | user-provided value |
433450
| tst3.js:12:12:12:15 | code | tst3.js:11:32:11:39 | reg.body | tst3.js:12:12:12:15 | code | Cross-site scripting vulnerability due to $@. | tst3.js:11:32:11:39 | reg.body | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,7 @@
4242
| tst2.js:51:12:51:17 | unsafe | Cross-site scripting vulnerability due to $@. | tst2.js:43:9:43:9 | p | user-provided value |
4343
| tst2.js:63:12:63:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:57:9:57:9 | p | user-provided value |
4444
| tst2.js:64:12:64:18 | other.p | Cross-site scripting vulnerability due to $@. | tst2.js:57:9:57:9 | p | user-provided value |
45+
| tst2.js:75:12:75:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:69:9:69:9 | p | user-provided value |
46+
| tst2.js:76:12:76:18 | other.p | Cross-site scripting vulnerability due to $@. | tst2.js:69:9:69:9 | p | user-provided value |
4547
| tst3.js:6:12:6:12 | p | Cross-site scripting vulnerability due to $@. | tst3.js:5:9:5:9 | p | user-provided value |
4648
| tst3.js:12:12:12:15 | code | Cross-site scripting vulnerability due to $@. | tst3.js:11:32:11:39 | reg.body | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/tst2.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,18 @@ app.get('/baz', function(req, res) {
6060
obj.p = p;
6161
var other = fclone(obj);
6262

63+
res.send(p); // NOT OK
64+
res.send(other.p); // NOT OK
65+
});
66+
67+
const jc = require('json-cycle');
68+
app.get('/baz', function(req, res) {
69+
let { p } = req.params;
70+
71+
var obj = {};
72+
obj.p = p;
73+
var other = jc.retrocycle(jc.decycle(obj));
74+
6375
res.send(p); // NOT OK
6476
res.send(other.p); // NOT OK
6577
});

0 commit comments

Comments
 (0)