From fd10947ca0f1f17d42436a7173c734f2d75cd37d Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Mon, 11 Jul 2022 16:22:54 +0200 Subject: [PATCH] use small steps in TypeBackTracker correctly --- .../lib/semmle/javascript/dataflow/TypeTracking.qll | 2 +- .../UnsafeHtmlConstructionCustomizations.qll | 2 +- .../src/Security/CWE-094/ImproperCodeSanitization.ql | 2 +- .../UnsafeHtmlConstruction.expected | 12 ++++++++++++ .../Security/CWE-079/UnsafeHtmlConstruction/main.js | 8 ++++++++ 5 files changed, 23 insertions(+), 3 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/TypeTracking.qll b/javascript/ql/lib/semmle/javascript/dataflow/TypeTracking.qll index 896f0177ec3..12aa8d09ed1 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/TypeTracking.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/TypeTracking.qll @@ -312,7 +312,7 @@ class TypeBackTracker extends TTypeBackTracker { * result = < some API call >.getArgument(< n >) * or * exists (DataFlow::TypeBackTracker t2 | - * t = t2.smallstep(result, myType(t2)) + * t2 = t.smallstep(result, myType(t2)) * ) * } * diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll index d594da271b8..fd549429e4a 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll @@ -80,7 +80,7 @@ module UnsafeHtmlConstruction { t.start() and result = sink or - exists(DataFlow::TypeBackTracker t2 | t = t2.smallstep(result, isUsedInXssSink(t2, sink))) + exists(DataFlow::TypeBackTracker t2 | t2 = t.smallstep(result, isUsedInXssSink(t2, sink))) or exists(DataFlow::TypeBackTracker t2 | t.continue() = t2 and diff --git a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql index 886c78b0161..694e827ba41 100644 --- a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql +++ b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql @@ -50,7 +50,7 @@ private DataFlow::Node endsInCodeInjectionSink(DataFlow::TypeBackTracker t) { not result instanceof StringOps::ConcatenationRoot // the heuristic CodeInjection sink looks for string-concats, we are not interrested in those here. ) or - exists(DataFlow::TypeBackTracker t2 | t = t2.smallstep(result, endsInCodeInjectionSink(t2))) + exists(DataFlow::TypeBackTracker t2 | t2 = t.smallstep(result, endsInCodeInjectionSink(t2))) } /** diff --git a/javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/UnsafeHtmlConstruction.expected b/javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/UnsafeHtmlConstruction.expected index c604f4ab2d2..94f1fe314b0 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/UnsafeHtmlConstruction.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/UnsafeHtmlConstruction.expected @@ -50,6 +50,12 @@ nodes | main.js:79:34:79:36 | val | | main.js:81:35:81:37 | val | | main.js:81:35:81:37 | val | +| main.js:89:21:89:21 | x | +| main.js:90:23:90:23 | x | +| main.js:90:23:90:23 | x | +| main.js:93:43:93:43 | x | +| main.js:93:43:93:43 | x | +| main.js:94:31:94:31 | x | | typed.ts:1:39:1:39 | s | | typed.ts:1:39:1:39 | s | | typed.ts:2:29:2:29 | s | @@ -115,6 +121,11 @@ edges | main.js:79:34:79:36 | val | main.js:81:35:81:37 | val | | main.js:79:34:79:36 | val | main.js:81:35:81:37 | val | | main.js:79:34:79:36 | val | main.js:81:35:81:37 | val | +| main.js:89:21:89:21 | x | main.js:90:23:90:23 | x | +| main.js:89:21:89:21 | x | main.js:90:23:90:23 | x | +| main.js:93:43:93:43 | x | main.js:94:31:94:31 | x | +| main.js:93:43:93:43 | x | main.js:94:31:94:31 | x | +| main.js:94:31:94:31 | x | main.js:89:21:89:21 | x | | typed.ts:1:39:1:39 | s | typed.ts:2:29:2:29 | s | | typed.ts:1:39:1:39 | s | typed.ts:2:29:2:29 | s | | typed.ts:1:39:1:39 | s | typed.ts:2:29:2:29 | s | @@ -141,5 +152,6 @@ edges | main.js:62:19:62:31 | settings.name | main.js:56:28:56:34 | options | main.js:62:19:62:31 | settings.name | $@ based on $@ might later cause $@. | main.js:62:19:62:31 | settings.name | HTML construction | main.js:56:28:56:34 | options | library input | main.js:62:11:62:40 | "" + ... "" | cross-site scripting | | main.js:67:63:67:69 | attrVal | main.js:66:35:66:41 | attrVal | main.js:67:63:67:69 | attrVal | $@ based on $@ might later cause $@. | main.js:67:63:67:69 | attrVal | HTML construction | main.js:66:35:66:41 | attrVal | library input | main.js:67:47:67:78 | "" | cross-site scripting | | main.js:81:35:81:37 | val | main.js:79:34:79:36 | val | main.js:81:35:81:37 | val | $@ based on $@ might later cause $@. | main.js:81:35:81:37 | val | HTML construction | main.js:79:34:79:36 | val | library input | main.js:81:24:81:49 | " ... /span>" | cross-site scripting | +| main.js:90:23:90:23 | x | main.js:93:43:93:43 | x | main.js:90:23:90:23 | x | $@ based on $@ might later cause $@. | main.js:90:23:90:23 | x | HTML construction | main.js:93:43:93:43 | x | library input | main.js:94:20:94:32 | createHTML(x) | cross-site scripting | | typed.ts:2:29:2:29 | s | typed.ts:1:39:1:39 | s | typed.ts:2:29:2:29 | s | $@ based on $@ might later cause $@. | typed.ts:2:29:2:29 | s | HTML construction | typed.ts:1:39:1:39 | s | library input | typed.ts:3:31:3:34 | html | cross-site scripting | | typed.ts:8:40:8:40 | s | typed.ts:6:43:6:43 | s | typed.ts:8:40:8:40 | s | $@ based on $@ might later cause $@. | typed.ts:8:40:8:40 | s | HTML construction | typed.ts:6:43:6:43 | s | library input | typed.ts:8:29:8:52 | " ... /span>" | cross-site scripting | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/main.js b/javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/main.js index 1547ae86b24..2e9d344b1f3 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/main.js +++ b/javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/main.js @@ -85,3 +85,11 @@ module.exports.types = function (val) { $("#foo").html("" + val + ""); // OK } } + +function createHTML(x) { + return "" + x + ""; // NOT OK +} + +module.exports.usesCreateHTML = function (x) { + $("#foo").html(createHTML(x)); +} \ No newline at end of file