From 06069f783b14550c50d2bc5087fe731134ba899b Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 9 Jan 2025 12:51:35 +0100 Subject: [PATCH 1/9] JS: Refactor domEventSource() into a Range class --- javascript/ql/lib/semmle/javascript/DOM.qll | 46 +++++++++++++-------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/DOM.qll b/javascript/ql/lib/semmle/javascript/DOM.qll index 3dbe734b0fb2..6c6d70fede87 100644 --- a/javascript/ql/lib/semmle/javascript/DOM.qll +++ b/javascript/ql/lib/semmle/javascript/DOM.qll @@ -388,23 +388,33 @@ module DOM { } } - /** - * Gets a reference to a DOM event. - */ - private DataFlow::SourceNode domEventSource() { - // e.g.
e.target}/> - exists(JsxAttribute attr | attr.getName().matches("on%") | - result = attr.getValue().flow().getABoundFunctionValue(0).getParameter(0) - ) - or - // node.addEventListener("submit", e => e.target) - result = domValueRef().getAMethodCall("addEventListener").getABoundCallbackParameter(1, 0) - or - // node.onSubmit = (e => e.target); - exists(DataFlow::PropWrite write | write = domValueRef().getAPropertyWrite() | - write.getPropertyName().matches("on%") and - result = write.getRhs().getAFunctionValue().getParameter(0) - ) + /** A data flow node that is a source of DOM events. */ + class DomEventSource extends DataFlow::Node instanceof DomEventSource::Range { } + + /** Companion module to the `DomEventSource` class. */ + module DomEventSource { + /** + * A data flow node that should be considered a source of DOM events. + */ + abstract class Range extends DataFlow::Node { } + + private class DefaultRange extends Range { + DefaultRange() { + // e.g. e.target}/> + exists(JsxAttribute attr | attr.getName().matches("on%") | + this = attr.getValue().flow().getABoundFunctionValue(0).getParameter(0) + ) + or + // node.addEventListener("submit", e => e.target) + this = domValueRef().getAMethodCall("addEventListener").getABoundCallbackParameter(1, 0) + or + // node.onSubmit = (e => e.target); + exists(DataFlow::PropWrite write | write = domValueRef().getAPropertyWrite() | + write.getPropertyName().matches("on%") and + this = write.getRhs().getAFunctionValue().getParameter(0) + ) + } + } } /** Gets a data flow node that refers directly to a value from the DOM. */ @@ -419,7 +429,7 @@ module DOM { result = domValueRef().getAMethodCall(["item", "namedItem"]) or t.startInProp("target") and - result = domEventSource() + result instanceof DomEventSource or t.startInProp(DataFlow::PseudoProperties::arrayElement()) and result = domElementCollection() From 530f72ec7c749a4c07f5750fa83bb834dc0f1e7a Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 9 Jan 2025 13:10:04 +0100 Subject: [PATCH 2/9] JS: Add Angular test case in XssThroughDom --- .../Security/CWE-079/XssThroughDom/angular.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts diff --git a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts new file mode 100644 index 000000000000..5c41d8c7c585 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts @@ -0,0 +1,17 @@ +import { Component } from "@angular/core"; + +@Component({ + template: ` + + + ` +}) +export class Foo { + setInput1(event) { + document.write(event.target.value); // NOT OK [INCONSISTENCY] + } + + setInput2(target) { + document.write(target.value); // NOT OK [INCONSISTENCY] + } +} From 2830605bf7dd8dc48d211fc7585f693ba2d306e5 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 9 Jan 2025 13:13:58 +0100 Subject: [PATCH 3/9] JS: Add DOM event sources in Angular2 model --- .../semmle/javascript/frameworks/Angular2.qll | 21 +++++++++++++++++++ .../XssThroughDom/XssThroughDom.expected | 10 +++++++++ .../Security/CWE-079/XssThroughDom/angular.ts | 4 ++-- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Angular2.qll b/javascript/ql/lib/semmle/javascript/frameworks/Angular2.qll index 16430ff0475a..3ac3d947aced 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Angular2.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Angular2.qll @@ -554,4 +554,25 @@ module Angular2 { this = API::Node::ofType("@angular/core", "ElementRef").getMember("nativeElement").asSource() } } + + /** + * A source of DOM events originating from the `$event` variable in an event handler installed in an Angular template. + */ + private class DomEventSources extends DOM::DomEventSource::Range { + DomEventSources() { + exists(HTML::Element elm, string attributeName | + elm = any(ComponentClass cls).getATemplateElement() and + // Ignore instantiations of known element (mainly focus on native DOM elements) + not elm = any(ComponentClass cls).getATemplateInstantiation() and + not elm.getName().matches("ng-%") and + this = + elm.getAttributeByName(attributeName) + .getCodeInAttribute() + .(TemplateTopLevel) + .getAVariableUse("$event") and + attributeName.matches("(%)") and // event handler attribute + not attributeName.matches("(ng%)") // exclude NG events which aren't necessarily DOM events + ) + } + } } 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 83147705499c..d14cdc7c0d07 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 @@ -1,4 +1,10 @@ nodes +| angular.ts:11:24:11:41 | event.target.value | +| angular.ts:11:24:11:41 | event.target.value | +| angular.ts:11:24:11:41 | event.target.value | +| angular.ts:15:24:15:35 | target.value | +| angular.ts:15:24:15:35 | target.value | +| angular.ts:15:24:15:35 | target.value | | forms.js:8:23:8:28 | values | | forms.js:8:23:8:28 | values | | forms.js:9:31:9:36 | values | @@ -165,6 +171,8 @@ nodes | xss-through-dom.js:159:34:159:52 | $("textarea").val() | | xss-through-dom.js:159:34:159:52 | $("textarea").val() | edges +| angular.ts:11:24:11:41 | event.target.value | angular.ts:11:24:11:41 | event.target.value | +| angular.ts:15:24:15:35 | target.value | angular.ts:15:24:15:35 | target.value | | 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 | | forms.js:9:31:9:36 | values | forms.js:9:31:9:40 | values.foo | @@ -273,6 +281,8 @@ edges | xss-through-dom.js:159:34:159:52 | $("textarea").val() | xss-through-dom.js:154:25:154:27 | msg | | xss-through-dom.js:159:34:159:52 | $("textarea").val() | xss-through-dom.js:154:25:154:27 | msg | #select +| angular.ts:11:24:11:41 | event.target.value | angular.ts:11:24:11:41 | event.target.value | angular.ts:11:24:11:41 | event.target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:11:24:11:41 | event.target.value | DOM text | +| angular.ts:15:24:15:35 | target.value | angular.ts:15:24:15:35 | target.value | angular.ts:15:24:15:35 | target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:15:24:15:35 | target.value | DOM text | | 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 | | forms.js:25:23:25:34 | values.email | forms.js:24:15:24:20 | values | forms.js:25:23:25:34 | values.email | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:24:15:24:20 | values | DOM text | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts index 5c41d8c7c585..837df66a89c3 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts +++ b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts @@ -8,10 +8,10 @@ import { Component } from "@angular/core"; }) export class Foo { setInput1(event) { - document.write(event.target.value); // NOT OK [INCONSISTENCY] + document.write(event.target.value); // NOT OK } setInput2(target) { - document.write(target.value); // NOT OK [INCONSISTENCY] + document.write(target.value); // NOT OK } } From 174e1083a38a33cc63c862b914e317171c2dd985 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 9 Jan 2025 13:24:01 +0100 Subject: [PATCH 4/9] JS: Add test with NgForm.value --- .../XssThroughDom/XssThroughDom.expected | 20 +++++++++---------- .../Security/CWE-079/XssThroughDom/angular.ts | 5 +++++ 2 files changed, 15 insertions(+), 10 deletions(-) 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 d14cdc7c0d07..6a70ff809d92 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 @@ -1,10 +1,10 @@ nodes -| angular.ts:11:24:11:41 | event.target.value | -| angular.ts:11:24:11:41 | event.target.value | -| angular.ts:11:24:11:41 | event.target.value | -| angular.ts:15:24:15:35 | target.value | -| angular.ts:15:24:15:35 | target.value | -| angular.ts:15:24:15:35 | target.value | +| angular.ts:12:24:12:41 | event.target.value | +| angular.ts:12:24:12:41 | event.target.value | +| angular.ts:12:24:12:41 | event.target.value | +| angular.ts:16:24:16:35 | target.value | +| angular.ts:16:24:16:35 | target.value | +| angular.ts:16:24:16:35 | target.value | | forms.js:8:23:8:28 | values | | forms.js:8:23:8:28 | values | | forms.js:9:31:9:36 | values | @@ -171,8 +171,8 @@ nodes | xss-through-dom.js:159:34:159:52 | $("textarea").val() | | xss-through-dom.js:159:34:159:52 | $("textarea").val() | edges -| angular.ts:11:24:11:41 | event.target.value | angular.ts:11:24:11:41 | event.target.value | -| angular.ts:15:24:15:35 | target.value | angular.ts:15:24:15:35 | target.value | +| angular.ts:12:24:12:41 | event.target.value | angular.ts:12:24:12:41 | event.target.value | +| angular.ts:16:24:16:35 | target.value | angular.ts:16:24:16:35 | target.value | | 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 | | forms.js:9:31:9:36 | values | forms.js:9:31:9:40 | values.foo | @@ -281,8 +281,8 @@ edges | xss-through-dom.js:159:34:159:52 | $("textarea").val() | xss-through-dom.js:154:25:154:27 | msg | | xss-through-dom.js:159:34:159:52 | $("textarea").val() | xss-through-dom.js:154:25:154:27 | msg | #select -| angular.ts:11:24:11:41 | event.target.value | angular.ts:11:24:11:41 | event.target.value | angular.ts:11:24:11:41 | event.target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:11:24:11:41 | event.target.value | DOM text | -| angular.ts:15:24:15:35 | target.value | angular.ts:15:24:15:35 | target.value | angular.ts:15:24:15:35 | target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:15:24:15:35 | target.value | DOM text | +| angular.ts:12:24:12:41 | event.target.value | angular.ts:12:24:12:41 | event.target.value | angular.ts:12:24:12:41 | event.target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:12:24:12:41 | event.target.value | DOM text | +| angular.ts:16:24:16:35 | target.value | angular.ts:16:24:16:35 | target.value | angular.ts:16:24:16:35 | target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:16:24:16:35 | target.value | DOM text | | 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 | | forms.js:25:23:25:34 | values.email | forms.js:24:15:24:20 | values | forms.js:25:23:25:34 | values.email | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:24:15:24:20 | values | DOM text | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts index 837df66a89c3..b5b59cb80a33 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts +++ b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts @@ -1,4 +1,5 @@ import { Component } from "@angular/core"; +import { NgForm } from "@angular/forms"; @Component({ template: ` @@ -14,4 +15,8 @@ export class Foo { setInput2(target) { document.write(target.value); // NOT OK } + + blah(form: NgForm) { + document.write(form.value.foo); // NOT OK [INCONSISTENCY] + } } From caa48e83dbaad1f08f81207a7c14965e2bebebe3 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 9 Jan 2025 13:27:38 +0100 Subject: [PATCH 5/9] JS: Recognise form input from NgForm --- .../security/dataflow/XssThroughDomCustomizations.qll | 9 +++++++++ .../CWE-079/XssThroughDom/XssThroughDom.expected | 9 +++++++++ .../Security/CWE-079/XssThroughDom/angular.ts | 2 +- 3 files changed, 19 insertions(+), 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 bf38b2e2a5d1..88f84d28b8eb 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomCustomizations.qll @@ -211,6 +211,15 @@ module XssThroughDom { ) } } + + /** + * An object containing input values from an Angular form, accessed through an `NgForm` object. + */ + class AngularFormSource extends Source { + AngularFormSource() { + this = API::Node::ofType("@angular/forms", "NgForm").getMember("value").asSource() + } + } } /** 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 6a70ff809d92..47714766b009 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 @@ -5,6 +5,10 @@ nodes | angular.ts:16:24:16:35 | target.value | | angular.ts:16:24:16:35 | target.value | | angular.ts:16:24:16:35 | target.value | +| angular.ts:20:24:20:33 | form.value | +| angular.ts:20:24:20:33 | form.value | +| angular.ts:20:24:20:37 | form.value.foo | +| angular.ts:20:24:20:37 | form.value.foo | | forms.js:8:23:8:28 | values | | forms.js:8:23:8:28 | values | | forms.js:9:31:9:36 | values | @@ -173,6 +177,10 @@ nodes edges | angular.ts:12:24:12:41 | event.target.value | angular.ts:12:24:12:41 | event.target.value | | angular.ts:16:24:16:35 | target.value | angular.ts:16:24:16:35 | target.value | +| angular.ts:20:24:20:33 | form.value | angular.ts:20:24:20:37 | form.value.foo | +| angular.ts:20:24:20:33 | form.value | angular.ts:20:24:20:37 | form.value.foo | +| angular.ts:20:24:20:33 | form.value | angular.ts:20:24:20:37 | form.value.foo | +| angular.ts:20:24:20:33 | form.value | angular.ts:20:24:20:37 | form.value.foo | | 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 | | forms.js:9:31:9:36 | values | forms.js:9:31:9:40 | values.foo | @@ -283,6 +291,7 @@ edges #select | angular.ts:12:24:12:41 | event.target.value | angular.ts:12:24:12:41 | event.target.value | angular.ts:12:24:12:41 | event.target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:12:24:12:41 | event.target.value | DOM text | | angular.ts:16:24:16:35 | target.value | angular.ts:16:24:16:35 | target.value | angular.ts:16:24:16:35 | target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:16:24:16:35 | target.value | DOM text | +| angular.ts:20:24:20:37 | form.value.foo | angular.ts:20:24:20:33 | form.value | angular.ts:20:24:20:37 | form.value.foo | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:20:24:20:33 | form.value | DOM text | | 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 | | forms.js:25:23:25:34 | values.email | forms.js:24:15:24:20 | values | forms.js:25:23:25:34 | values.email | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:24:15:24:20 | values | DOM text | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts index b5b59cb80a33..a8ba9377336a 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts +++ b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts @@ -17,6 +17,6 @@ export class Foo { } blah(form: NgForm) { - document.write(form.value.foo); // NOT OK [INCONSISTENCY] + document.write(form.value.foo); // NOT OK } } From ec4245f404b67f53aa667a745c64cc71decd0403 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 9 Jan 2025 13:29:13 +0100 Subject: [PATCH 6/9] JS: Add test case with [(ngModel)] --- .../XssThroughDom/XssThroughDom.expected | 38 +++++++++---------- .../Security/CWE-079/XssThroughDom/angular.ts | 7 ++++ 2 files changed, 26 insertions(+), 19 deletions(-) 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 47714766b009..cb91eb81235a 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 @@ -1,14 +1,14 @@ nodes -| angular.ts:12:24:12:41 | event.target.value | -| angular.ts:12:24:12:41 | event.target.value | -| angular.ts:12:24:12:41 | event.target.value | -| angular.ts:16:24:16:35 | target.value | -| angular.ts:16:24:16:35 | target.value | -| angular.ts:16:24:16:35 | target.value | -| angular.ts:20:24:20:33 | form.value | -| angular.ts:20:24:20:33 | form.value | -| angular.ts:20:24:20:37 | form.value.foo | -| angular.ts:20:24:20:37 | form.value.foo | +| angular.ts:15:24:15:41 | event.target.value | +| angular.ts:15:24:15:41 | event.target.value | +| angular.ts:15:24:15:41 | event.target.value | +| angular.ts:19:24:19:35 | target.value | +| angular.ts:19:24:19:35 | target.value | +| angular.ts:19:24:19:35 | target.value | +| angular.ts:23:24:23:33 | form.value | +| angular.ts:23:24:23:33 | form.value | +| angular.ts:23:24:23:37 | form.value.foo | +| angular.ts:23:24:23:37 | form.value.foo | | forms.js:8:23:8:28 | values | | forms.js:8:23:8:28 | values | | forms.js:9:31:9:36 | values | @@ -175,12 +175,12 @@ nodes | xss-through-dom.js:159:34:159:52 | $("textarea").val() | | xss-through-dom.js:159:34:159:52 | $("textarea").val() | edges -| angular.ts:12:24:12:41 | event.target.value | angular.ts:12:24:12:41 | event.target.value | -| angular.ts:16:24:16:35 | target.value | angular.ts:16:24:16:35 | target.value | -| angular.ts:20:24:20:33 | form.value | angular.ts:20:24:20:37 | form.value.foo | -| angular.ts:20:24:20:33 | form.value | angular.ts:20:24:20:37 | form.value.foo | -| angular.ts:20:24:20:33 | form.value | angular.ts:20:24:20:37 | form.value.foo | -| angular.ts:20:24:20:33 | form.value | angular.ts:20:24:20:37 | form.value.foo | +| angular.ts:15:24:15:41 | event.target.value | angular.ts:15:24:15:41 | event.target.value | +| angular.ts:19:24:19:35 | target.value | angular.ts:19:24:19:35 | target.value | +| angular.ts:23:24:23:33 | form.value | angular.ts:23:24:23:37 | form.value.foo | +| angular.ts:23:24:23:33 | form.value | angular.ts:23:24:23:37 | form.value.foo | +| angular.ts:23:24:23:33 | form.value | angular.ts:23:24:23:37 | form.value.foo | +| angular.ts:23:24:23:33 | form.value | angular.ts:23:24:23:37 | form.value.foo | | 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 | | forms.js:9:31:9:36 | values | forms.js:9:31:9:40 | values.foo | @@ -289,9 +289,9 @@ edges | xss-through-dom.js:159:34:159:52 | $("textarea").val() | xss-through-dom.js:154:25:154:27 | msg | | xss-through-dom.js:159:34:159:52 | $("textarea").val() | xss-through-dom.js:154:25:154:27 | msg | #select -| angular.ts:12:24:12:41 | event.target.value | angular.ts:12:24:12:41 | event.target.value | angular.ts:12:24:12:41 | event.target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:12:24:12:41 | event.target.value | DOM text | -| angular.ts:16:24:16:35 | target.value | angular.ts:16:24:16:35 | target.value | angular.ts:16:24:16:35 | target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:16:24:16:35 | target.value | DOM text | -| angular.ts:20:24:20:37 | form.value.foo | angular.ts:20:24:20:33 | form.value | angular.ts:20:24:20:37 | form.value.foo | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:20:24:20:33 | form.value | DOM text | +| angular.ts:15:24:15:41 | event.target.value | angular.ts:15:24:15:41 | event.target.value | angular.ts:15:24:15:41 | event.target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:15:24:15:41 | event.target.value | DOM text | +| angular.ts:19:24:19:35 | target.value | angular.ts:19:24:19:35 | target.value | angular.ts:19:24:19:35 | target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:19:24:19:35 | target.value | DOM text | +| angular.ts:23:24:23:37 | form.value.foo | angular.ts:23:24:23:33 | form.value | angular.ts:23:24:23:37 | form.value.foo | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:23:24:23:33 | form.value | DOM text | | 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 | | forms.js:25:23:25:34 | values.email | forms.js:24:15:24:20 | values | forms.js:25:23:25:34 | values.email | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:24:15:24:20 | values | DOM text | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts index a8ba9377336a..1bad99ae0450 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts +++ b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts @@ -5,9 +5,12 @@ import { NgForm } from "@angular/forms"; template: ` + ` }) export class Foo { + field: string = ""; + setInput1(event) { document.write(event.target.value); // NOT OK } @@ -19,4 +22,8 @@ export class Foo { blah(form: NgForm) { document.write(form.value.foo); // NOT OK } + + useField() { + document.write(this.field); // NOT OK [INCONSISTENCY] + } } From dc8743b4ae804d2df45d537eda2e951b90e07830 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 9 Jan 2025 13:47:00 +0100 Subject: [PATCH 7/9] JS: Support [(ngModel)] --- .../src/com/semmle/js/extractor/HTMLExtractor.java | 2 +- .../dataflow/XssThroughDomCustomizations.qll | 12 ++++++++++++ .../CWE-079/XssThroughDom/XssThroughDom.expected | 9 +++++++++ .../Security/CWE-079/XssThroughDom/angular.ts | 2 +- 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/javascript/extractor/src/com/semmle/js/extractor/HTMLExtractor.java b/javascript/extractor/src/com/semmle/js/extractor/HTMLExtractor.java index bcc0e4b6af63..9e66e3232eb4 100644 --- a/javascript/extractor/src/com/semmle/js/extractor/HTMLExtractor.java +++ b/javascript/extractor/src/com/semmle/js/extractor/HTMLExtractor.java @@ -186,7 +186,7 @@ private boolean isAngularTemplateAttributeName(String name) { /** Attribute names that look valid in HTML or in one of the template languages we support, like Vue and Angular. */ private static final Pattern VALID_ATTRIBUTE_NAME = - Pattern.compile("[*:@]?\\[?\\(?[\\w:_\\-.]+\\]?\\)?"); + Pattern.compile("[*:@]?\\[?\\(?[\\w:_\\-.]+\\)?\\]?"); /** List of HTML attributes whose value is interpreted as JavaScript. */ private static final Pattern JS_ATTRIBUTE = diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomCustomizations.qll index 88f84d28b8eb..7d3b0e2c74e2 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomCustomizations.qll @@ -249,4 +249,16 @@ module XssThroughDom { this = getSelectionCall(DataFlow::TypeTracker::end()).getAMethodCall("toString") } } + + /** + * A source of DOM input originating from an Angular two-way data binding. + */ + private class AngularNgModelSource extends Source { + AngularNgModelSource() { + exists(Angular2::ComponentClass component, string fieldName | + fieldName = component.getATemplateElement().getAttributeByName("[(ngModel)]").getValue() and + this = component.getFieldInputNode(fieldName) + ) + } + } } 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 cb91eb81235a..96a4f0cd2dfe 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 @@ -1,4 +1,6 @@ nodes +| angular.ts:12:5:12:23 | field: string = ""; | +| angular.ts:12:5:12:23 | field: string = ""; | | angular.ts:15:24:15:41 | event.target.value | | angular.ts:15:24:15:41 | event.target.value | | angular.ts:15:24:15:41 | event.target.value | @@ -9,6 +11,8 @@ nodes | angular.ts:23:24:23:33 | form.value | | angular.ts:23:24:23:37 | form.value.foo | | angular.ts:23:24:23:37 | form.value.foo | +| angular.ts:27:24:27:33 | this.field | +| angular.ts:27:24:27:33 | this.field | | forms.js:8:23:8:28 | values | | forms.js:8:23:8:28 | values | | forms.js:9:31:9:36 | values | @@ -175,6 +179,10 @@ nodes | xss-through-dom.js:159:34:159:52 | $("textarea").val() | | xss-through-dom.js:159:34:159:52 | $("textarea").val() | edges +| angular.ts:12:5:12:23 | field: string = ""; | angular.ts:27:24:27:33 | this.field | +| angular.ts:12:5:12:23 | field: string = ""; | angular.ts:27:24:27:33 | this.field | +| angular.ts:12:5:12:23 | field: string = ""; | angular.ts:27:24:27:33 | this.field | +| angular.ts:12:5:12:23 | field: string = ""; | angular.ts:27:24:27:33 | this.field | | angular.ts:15:24:15:41 | event.target.value | angular.ts:15:24:15:41 | event.target.value | | angular.ts:19:24:19:35 | target.value | angular.ts:19:24:19:35 | target.value | | angular.ts:23:24:23:33 | form.value | angular.ts:23:24:23:37 | form.value.foo | @@ -292,6 +300,7 @@ edges | angular.ts:15:24:15:41 | event.target.value | angular.ts:15:24:15:41 | event.target.value | angular.ts:15:24:15:41 | event.target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:15:24:15:41 | event.target.value | DOM text | | angular.ts:19:24:19:35 | target.value | angular.ts:19:24:19:35 | target.value | angular.ts:19:24:19:35 | target.value | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:19:24:19:35 | target.value | DOM text | | angular.ts:23:24:23:37 | form.value.foo | angular.ts:23:24:23:33 | form.value | angular.ts:23:24:23:37 | form.value.foo | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:23:24:23:33 | form.value | DOM text | +| angular.ts:27:24:27:33 | this.field | angular.ts:12:5:12:23 | field: string = ""; | angular.ts:27:24:27:33 | this.field | $@ is reinterpreted as HTML without escaping meta-characters. | angular.ts:12:5:12:23 | field: string = ""; | DOM text | | 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 | | forms.js:25:23:25:34 | values.email | forms.js:24:15:24:20 | values | forms.js:25:23:25:34 | values.email | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:24:15:24:20 | values | DOM text | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts index 1bad99ae0450..ad44929b1c76 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts +++ b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts @@ -24,6 +24,6 @@ export class Foo { } useField() { - document.write(this.field); // NOT OK [INCONSISTENCY] + document.write(this.field); // NOT OK } } From 95be339976afdc418bdbd2489999c98ee28b48d6 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 9 Jan 2025 13:51:30 +0100 Subject: [PATCH 8/9] JS: Bump extractor version string --- javascript/extractor/src/com/semmle/js/extractor/Main.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/extractor/src/com/semmle/js/extractor/Main.java b/javascript/extractor/src/com/semmle/js/extractor/Main.java index 4b2575b47f31..b455a2c9e099 100644 --- a/javascript/extractor/src/com/semmle/js/extractor/Main.java +++ b/javascript/extractor/src/com/semmle/js/extractor/Main.java @@ -41,7 +41,7 @@ public class Main { * A version identifier that should be updated every time the extractor changes in such a way that * it may produce different tuples for the same file under the same {@link ExtractorConfig}. */ - public static final String EXTRACTOR_VERSION = "2024-10-29"; + public static final String EXTRACTOR_VERSION = "2025-01-09"; public static final Pattern NEWLINE = Pattern.compile("\n"); From 19f8d62ff98d8b9afdeb4ada5f238b165966d763 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 9 Jan 2025 13:49:56 +0100 Subject: [PATCH 9/9] JS: Add change note --- .../src/change-notes/2025-01-09-angular2-xss-through-dom.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 javascript/ql/src/change-notes/2025-01-09-angular2-xss-through-dom.md diff --git a/javascript/ql/src/change-notes/2025-01-09-angular2-xss-through-dom.md b/javascript/ql/src/change-notes/2025-01-09-angular2-xss-through-dom.md new file mode 100644 index 000000000000..b88a3ca3de65 --- /dev/null +++ b/javascript/ql/src/change-notes/2025-01-09-angular2-xss-through-dom.md @@ -0,0 +1,4 @@ +--- +category: majorAnalysis +--- +* The `js/xss-through-dom` query now recognises sources of DOM input originating from Angular templates.