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/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"); 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() 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/lib/semmle/javascript/security/dataflow/XssThroughDomCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomCustomizations.qll index bf38b2e2a5d1..7d3b0e2c74e2 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() + } + } } /** @@ -240,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/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. 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..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,18 @@ 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 | +| 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 | +| 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 | @@ -165,6 +179,16 @@ 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 | +| 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 | @@ -273,6 +297,10 @@ 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: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 new file mode 100644 index 000000000000..ad44929b1c76 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/angular.ts @@ -0,0 +1,29 @@ +import { Component } from "@angular/core"; +import { NgForm } from "@angular/forms"; + +@Component({ + template: ` + + + + ` +}) +export class Foo { + field: string = ""; + + setInput1(event) { + document.write(event.target.value); // NOT OK + } + + setInput2(target) { + document.write(target.value); // NOT OK + } + + blah(form: NgForm) { + document.write(form.value.foo); // NOT OK + } + + useField() { + document.write(this.field); // NOT OK + } +}