From 285c659541158bda3449f774482adff7b1d83954 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Wed, 25 Aug 2021 12:54:15 +0200 Subject: [PATCH 1/3] add `src` as a potential unsafe DOM property name for `js/xss-through-dom` --- .../security/dataflow/XssThroughDomCustomizations.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomCustomizations.qll index efedee1477d9..d9222337d34e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomCustomizations.qll @@ -30,7 +30,7 @@ module XssThroughDom { /** * Gets a DOM property name that could store user-controlled data. */ - string unsafeDomPropertyName() { result = ["innerText", "textContent", "value", "name"] } + string unsafeDomPropertyName() { result = ["innerText", "textContent", "value", "name", "src"] } /** * A source for text from the DOM from a JQuery method call. From 1b6e1dbd13f7b0a0e5c5c27398c94c97cbcde218 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Wed, 25 Aug 2021 12:54:42 +0200 Subject: [PATCH 2/3] include property writes in super-classes when reading a property in a sub-class --- javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll b/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll index c9061b5c5b94..b5d1290a516d 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll @@ -1591,7 +1591,7 @@ module DataFlow { */ predicate localFieldStep(DataFlow::Node pred, DataFlow::Node succ) { exists(ClassNode cls, string prop | - pred = cls.getAReceiverNode().getAPropertyWrite(prop).getRhs() or + pred = cls.getADirectSuperClass*().getAReceiverNode().getAPropertyWrite(prop).getRhs() or pred = cls.getInstanceMethod(prop) | succ = cls.getAReceiverNode().getAPropertyRead(prop) From 81742528a2c5167500a7529937cf6b4f53def70b Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Wed, 25 Aug 2021 12:55:06 +0200 Subject: [PATCH 3/3] add test --- .../CWE-079/XssThroughDom/XssThroughDom.expected | 9 +++++++++ .../CWE-079/XssThroughDom/xss-through-dom.js | 15 ++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/XssThroughDom.expected b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/XssThroughDom.expected index 5d0884817116..032446f4935f 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/XssThroughDom.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/XssThroughDom.expected @@ -118,6 +118,10 @@ nodes | xss-through-dom.js:96:17:96:47 | $("#foo ... ].value | | xss-through-dom.js:96:17:96:47 | $("#foo ... ].value | | xss-through-dom.js:96:17:96:47 | $("#foo ... ].value | +| xss-through-dom.js:109:31:109:70 | "" | +| xss-through-dom.js:109:31:109:70 | "" | +| xss-through-dom.js:109:45:109:55 | this.el.src | +| xss-through-dom.js:109:45:109:55 | this.el.src | edges | forms.js:8:23:8:28 | values | forms.js:9:31:9:36 | values | | forms.js:8:23:8:28 | values | forms.js:9:31:9:36 | values | @@ -186,6 +190,10 @@ edges | xss-through-dom.js:87:36:87:39 | text | xss-through-dom.js:87:16:87:40 | new ans ... s(text) | | xss-through-dom.js:93:16:93:46 | $("#foo ... ].value | xss-through-dom.js:93:16:93:46 | $("#foo ... ].value | | xss-through-dom.js:96:17:96:47 | $("#foo ... ].value | xss-through-dom.js:96:17:96:47 | $("#foo ... ].value | +| xss-through-dom.js:109:45:109:55 | this.el.src | xss-through-dom.js:109:31:109:70 | "" | +| xss-through-dom.js:109:45:109:55 | this.el.src | xss-through-dom.js:109:31:109:70 | "" | +| xss-through-dom.js:109:45:109:55 | this.el.src | xss-through-dom.js:109:31:109:70 | "" | +| xss-through-dom.js:109:45:109:55 | this.el.src | xss-through-dom.js:109:31:109:70 | "" | #select | forms.js:9:31:9:40 | values.foo | forms.js:8:23:8:28 | values | forms.js:9:31:9:40 | values.foo | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:8:23:8:28 | values | DOM text | | forms.js:12:31:12:40 | values.bar | forms.js:11:24:11:29 | values | forms.js:12:31:12:40 | values.bar | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:11:24:11:29 | values | DOM text | @@ -219,3 +227,4 @@ edges | xss-through-dom.js:87:16:87:40 | new ans ... s(text) | xss-through-dom.js:84:15:84:30 | $("text").text() | xss-through-dom.js:87:16:87:40 | new ans ... s(text) | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:84:15:84:30 | $("text").text() | DOM text | | xss-through-dom.js:93:16:93:46 | $("#foo ... ].value | xss-through-dom.js:93:16:93:46 | $("#foo ... ].value | xss-through-dom.js:93:16:93:46 | $("#foo ... ].value | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:93:16:93:46 | $("#foo ... ].value | DOM text | | xss-through-dom.js:96:17:96:47 | $("#foo ... ].value | xss-through-dom.js:96:17:96:47 | $("#foo ... ].value | xss-through-dom.js:96:17:96:47 | $("#foo ... ].value | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:96:17:96:47 | $("#foo ... ].value | DOM text | +| xss-through-dom.js:109:31:109:70 | "" | xss-through-dom.js:109:45:109:55 | this.el.src | xss-through-dom.js:109:31:109:70 | "" | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:109:45:109:55 | this.el.src | DOM text | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/xss-through-dom.js b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/xss-through-dom.js index d7f6d6a8208d..b6e23c11c021 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/xss-through-dom.js +++ b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/xss-through-dom.js @@ -95,4 +95,17 @@ for (var i = 0; i < foo.length; i++) { $("#id").html($("#foo").find(".bla")[i].value); // NOT OK. } -})(); \ No newline at end of file +})(); + +class Super { + constructor() { + this.el = $("#id").get(0); + } +} + +class Sub extends Super { + constructor() { + super(); + $("#id").get(0).innerHTML = "foo"; // NOT OK. Attack: `` + } +} \ No newline at end of file