From 09e4c78b0f766aa14d0863d77d0465c4fe63f7df Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Fri, 3 Jan 2025 16:33:42 +0000 Subject: [PATCH 01/14] New XSS sink - writing to innerHTML using the Angular Renderer2 API --- .../dataflow/DomBasedXssCustomizations.qll | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll index 72d9ae4e55a6..270d58d4fa7e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll @@ -251,6 +251,26 @@ module DomBasedXss { } } + /** + * A write to the `innerHTML` property of a DOM element, viewed as an XSS sink. + * + * Uses the Angular Renderer2 API, instead of the default `Element.innerHTML` property. + */ + class AngularRender2SetPropertyInnerHtmlSink extends Sink { + AngularRender2SetPropertyInnerHtmlSink() { + exists(API::CallNode setProperty | + setProperty = + API::moduleImport("@angular/core") + .getMember("Renderer2") + .getInstance() + .getMember("setProperty") + .getACall() and + this = setProperty.getParameter(2).asSink() and + setProperty.getParameter(1).asSink().asExpr().(StringLiteral).getValue() = "innerHTML" + ) + } + } + /** * A value being piped into the `safe` pipe in a template file, * disabling subsequent HTML escaping. From 0f648223562adaaffa6eea006263ee51a7cfcfb2 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Fri, 3 Jan 2025 16:34:15 +0000 Subject: [PATCH 02/14] New remote source - reading from an @Input() decorated class member --- .../security/dataflow/RemoteFlowSources.qll | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll index aad00b2d22e5..a41b8d8062d3 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll @@ -184,3 +184,36 @@ private class ExternalRemoteFlowSource extends RemoteFlowSource { override string getSourceType() { result = ap.getSourceType() } } + +// Angular @Input() decorator on a member declaration. +class InputMember extends MemberDeclaration { + InputMember() { + exists(Decorator decorator, Expr expr | + decorator.getElement() = this + and decorator.getExpression() = expr + and expr.(CallExpr).getCallee().(VarRef).getName() = "Input" + ) + } +} + +// Use of an Angular @Input() member. +class InputMemberUse extends DataFlow::Node { + InputMemberUse() { + exists(InputMember member, string memberName, ThisExpr ta, FieldAccess fa | + memberName = member.getName() + and fa.getBase() = ta + and fa.getPropertyName() = memberName + and this.asExpr() = fa + ) + } +} + +private class AngularInputUse extends RemoteFlowSource { + AngularInputUse() { + exists( InputMemberUse inputUse | + this = inputUse + ) + } + + override string getSourceType() { result = "Angular @Input()" } +} \ No newline at end of file From 477391787679b63672c597aacb530b8926150e8d Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Fri, 3 Jan 2025 16:43:00 +0000 Subject: [PATCH 03/14] Formatting --- .../dataflow/DomBasedXssCustomizations.qll | 2 +- .../security/dataflow/RemoteFlowSources.qll | 30 ++++++++----------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll index 270d58d4fa7e..ab5bff5d73e9 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll @@ -253,7 +253,7 @@ module DomBasedXss { /** * A write to the `innerHTML` property of a DOM element, viewed as an XSS sink. - * + * * Uses the Angular Renderer2 API, instead of the default `Element.innerHTML` property. */ class AngularRender2SetPropertyInnerHtmlSink extends Sink { diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll index a41b8d8062d3..6b1748996e3b 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll @@ -189,31 +189,27 @@ private class ExternalRemoteFlowSource extends RemoteFlowSource { class InputMember extends MemberDeclaration { InputMember() { exists(Decorator decorator, Expr expr | - decorator.getElement() = this - and decorator.getExpression() = expr - and expr.(CallExpr).getCallee().(VarRef).getName() = "Input" + decorator.getElement() = this and + decorator.getExpression() = expr and + expr.(CallExpr).getCallee().(VarRef).getName() = "Input" ) } } // Use of an Angular @Input() member. class InputMemberUse extends DataFlow::Node { - InputMemberUse() { - exists(InputMember member, string memberName, ThisExpr ta, FieldAccess fa | - memberName = member.getName() - and fa.getBase() = ta - and fa.getPropertyName() = memberName - and this.asExpr() = fa - ) - } + InputMemberUse() { + exists(InputMember member, string memberName, ThisExpr ta, FieldAccess fa | + memberName = member.getName() and + fa.getBase() = ta and + fa.getPropertyName() = memberName and + this.asExpr() = fa + ) + } } private class AngularInputUse extends RemoteFlowSource { - AngularInputUse() { - exists( InputMemberUse inputUse | - this = inputUse - ) - } + AngularInputUse() { exists(InputMemberUse inputUse | this = inputUse) } override string getSourceType() { result = "Angular @Input()" } -} \ No newline at end of file +} From 4891c1e5fe132df851c4ed333176047bab24883b Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Fri, 3 Jan 2025 16:50:47 +0000 Subject: [PATCH 04/14] Added QLdoc and simplified QL in source class --- .../security/dataflow/RemoteFlowSources.qll | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll index 6b1748996e3b..2c16406cac49 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll @@ -185,7 +185,9 @@ private class ExternalRemoteFlowSource extends RemoteFlowSource { override string getSourceType() { result = ap.getSourceType() } } -// Angular @Input() decorator on a member declaration. +/** + * Angular @Input() decorator on a member declaration. + */ class InputMember extends MemberDeclaration { InputMember() { exists(Decorator decorator, Expr expr | @@ -196,7 +198,9 @@ class InputMember extends MemberDeclaration { } } -// Use of an Angular @Input() member. +/** + * Use of an Angular @Input() member, modelled as `InputMember`. + */ class InputMemberUse extends DataFlow::Node { InputMemberUse() { exists(InputMember member, string memberName, ThisExpr ta, FieldAccess fa | @@ -208,8 +212,11 @@ class InputMemberUse extends DataFlow::Node { } } +/** + * A remote flow source that is a member of an Angular component class. + */ private class AngularInputUse extends RemoteFlowSource { - AngularInputUse() { exists(InputMemberUse inputUse | this = inputUse) } + AngularInputUse() { this instanceof InputMemberUse } override string getSourceType() { result = "Angular @Input()" } } From 712870000307ce275360bd9a05ebf72106294c77 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Fri, 3 Jan 2025 17:02:55 +0000 Subject: [PATCH 05/14] Simplified AngularInputUse class --- .../javascript/security/dataflow/RemoteFlowSources.qll | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll index 2c16406cac49..60031d3a94bb 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll @@ -186,7 +186,7 @@ private class ExternalRemoteFlowSource extends RemoteFlowSource { } /** - * Angular @Input() decorator on a member declaration. + * An Angular @Input() decorator on a member declaration. */ class InputMember extends MemberDeclaration { InputMember() { @@ -199,7 +199,7 @@ class InputMember extends MemberDeclaration { } /** - * Use of an Angular @Input() member, modelled as `InputMember`. + * A use of an Angular @Input() member, modeled as `InputMember`. */ class InputMemberUse extends DataFlow::Node { InputMemberUse() { @@ -215,8 +215,8 @@ class InputMemberUse extends DataFlow::Node { /** * A remote flow source that is a member of an Angular component class. */ -private class AngularInputUse extends RemoteFlowSource { - AngularInputUse() { this instanceof InputMemberUse } +private class AngularInputUse extends RemoteFlowSource, InputMemberUse { + AngularInputUse() { this = this } override string getSourceType() { result = "Angular @Input()" } } From aba8be2902b207cd8639882b3ad7b68e0598bc63 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Fri, 3 Jan 2025 17:07:35 +0000 Subject: [PATCH 06/14] Changelog for Angular source/sink update --- .../ql/lib/change-notes/2025-01-03-angular-source-sink.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 javascript/ql/lib/change-notes/2025-01-03-angular-source-sink.md diff --git a/javascript/ql/lib/change-notes/2025-01-03-angular-source-sink.md b/javascript/ql/lib/change-notes/2025-01-03-angular-source-sink.md new file mode 100644 index 000000000000..4ba7122ecccb --- /dev/null +++ b/javascript/ql/lib/change-notes/2025-01-03-angular-source-sink.md @@ -0,0 +1,5 @@ +--- +category: majorAnalysis +--- +* Added new remote source from class members decorated with `@Input()` +* Added new XSS sink where `InnerHTML` is assigned to with the Angular Renderer2 API From 8dac00aa832c55bd91d54544b310ad208810da7f Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Mon, 6 Jan 2025 15:43:47 +0000 Subject: [PATCH 07/14] Change from getParameter() to getArgument() --- .../javascript/security/dataflow/DomBasedXssCustomizations.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll index ab5bff5d73e9..8a3e66e8e806 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll @@ -266,7 +266,7 @@ module DomBasedXss { .getMember("setProperty") .getACall() and this = setProperty.getParameter(2).asSink() and - setProperty.getParameter(1).asSink().asExpr().(StringLiteral).getValue() = "innerHTML" + setProperty.getArgument(1).getStringValue() = "innerHTML" ) } } From e414b8c5be491c957f93526224b51a07c5a73f44 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Mon, 6 Jan 2025 16:51:35 +0000 Subject: [PATCH 08/14] Remove @Input() decorated members as remote sources, in favour of a later Threat Model --- .../security/dataflow/RemoteFlowSources.qll | 36 ------------------- 1 file changed, 36 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll index 60031d3a94bb..aad00b2d22e5 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll @@ -184,39 +184,3 @@ private class ExternalRemoteFlowSource extends RemoteFlowSource { override string getSourceType() { result = ap.getSourceType() } } - -/** - * An Angular @Input() decorator on a member declaration. - */ -class InputMember extends MemberDeclaration { - InputMember() { - exists(Decorator decorator, Expr expr | - decorator.getElement() = this and - decorator.getExpression() = expr and - expr.(CallExpr).getCallee().(VarRef).getName() = "Input" - ) - } -} - -/** - * A use of an Angular @Input() member, modeled as `InputMember`. - */ -class InputMemberUse extends DataFlow::Node { - InputMemberUse() { - exists(InputMember member, string memberName, ThisExpr ta, FieldAccess fa | - memberName = member.getName() and - fa.getBase() = ta and - fa.getPropertyName() = memberName and - this.asExpr() = fa - ) - } -} - -/** - * A remote flow source that is a member of an Angular component class. - */ -private class AngularInputUse extends RemoteFlowSource, InputMemberUse { - AngularInputUse() { this = this } - - override string getSourceType() { result = "Angular @Input()" } -} From 6fb201372bc95eee4f797d437cb1de6d9fdd18ab Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Mon, 6 Jan 2025 16:51:59 +0000 Subject: [PATCH 09/14] Update changelog note to remove new source --- javascript/ql/lib/change-notes/2025-01-03-angular-source-sink.md | 1 - 1 file changed, 1 deletion(-) diff --git a/javascript/ql/lib/change-notes/2025-01-03-angular-source-sink.md b/javascript/ql/lib/change-notes/2025-01-03-angular-source-sink.md index 4ba7122ecccb..609642c25b4a 100644 --- a/javascript/ql/lib/change-notes/2025-01-03-angular-source-sink.md +++ b/javascript/ql/lib/change-notes/2025-01-03-angular-source-sink.md @@ -1,5 +1,4 @@ --- category: majorAnalysis --- -* Added new remote source from class members decorated with `@Input()` * Added new XSS sink where `InnerHTML` is assigned to with the Angular Renderer2 API From 322c731ac339b6843f4ed58e8f55f8e9d5b28c21 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Mon, 6 Jan 2025 16:52:38 +0000 Subject: [PATCH 10/14] Attempt at AttributeDefinition to generalise Angular Renderer2 support --- .../frameworks/AngularJS/AngularJSCore.qll | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll b/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll index 1a6d11cd7534..2778a9f84d78 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll @@ -1032,3 +1032,37 @@ private class BindCall extends DataFlow::PartialInvokeNode::Range, DataFlow::Cal result = this.getArgument(0) } } + +/** + * A DOM attribute write, using the AngularJS Renderer2 API: a call to `Renderer2.setProperty`. + */ +private class AngularRenderer2AttributeDefinition extends DOM::AttributeDefinition { + DataFlow::Node propertyNode; + DataFlow::Node valueNode; + DataFlow::Node elementNode; + + AngularRenderer2AttributeDefinition() { + exists(API::CallNode setProperty | + setProperty = + API::moduleImport("@angular/core") + .getMember("Renderer2") + .getInstance() + .getMember("setProperty") + .getACall() and + elementNode = setProperty.getArgument(0) and + propertyNode = setProperty.getArgument(1) and + valueNode = setProperty.getArgument(2) and + this = setProperty.asExpr() + ) + } + + override string getName() { result = propertyNode.getStringValue() } + + // override DOM::ElementDefinition getElement() { /* TODO */ } + + DataFlow::Node getElementNode() { result = elementNode } + + override DataFlow::Node getValueNode() { result = valueNode } + + //override predicate mayHaveTemplateValue() { /* TODO */ } +} From 820fe6cd042385058f1a25f0b22fc389078098de Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Mon, 6 Jan 2025 16:59:04 +0000 Subject: [PATCH 11/14] Formatting --- .../semmle/javascript/frameworks/AngularJS/AngularJSCore.qll | 2 -- 1 file changed, 2 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll b/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll index 2778a9f84d78..2b4826c1529b 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll @@ -1059,10 +1059,8 @@ private class AngularRenderer2AttributeDefinition extends DOM::AttributeDefiniti override string getName() { result = propertyNode.getStringValue() } // override DOM::ElementDefinition getElement() { /* TODO */ } - DataFlow::Node getElementNode() { result = elementNode } override DataFlow::Node getValueNode() { result = valueNode } - //override predicate mayHaveTemplateValue() { /* TODO */ } } From 45301186810dc0b41b596fbda246d0ef00b41189 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Mon, 6 Jan 2025 17:33:31 +0000 Subject: [PATCH 12/14] Comment out hardcoded definition of sink --- .../dataflow/DomBasedXssCustomizations.qll | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll index 8a3e66e8e806..026cf47106fd 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll @@ -251,25 +251,25 @@ module DomBasedXss { } } - /** - * A write to the `innerHTML` property of a DOM element, viewed as an XSS sink. - * - * Uses the Angular Renderer2 API, instead of the default `Element.innerHTML` property. - */ - class AngularRender2SetPropertyInnerHtmlSink extends Sink { - AngularRender2SetPropertyInnerHtmlSink() { - exists(API::CallNode setProperty | - setProperty = - API::moduleImport("@angular/core") - .getMember("Renderer2") - .getInstance() - .getMember("setProperty") - .getACall() and - this = setProperty.getParameter(2).asSink() and - setProperty.getArgument(1).getStringValue() = "innerHTML" - ) - } - } + // /** + // * A write to the `innerHTML` property of a DOM element, viewed as an XSS sink. + // * + // * Uses the Angular Renderer2 API, instead of the default `Element.innerHTML` property. + // */ + // class AngularRender2SetPropertyInnerHtmlSink extends Sink { + // AngularRender2SetPropertyInnerHtmlSink() { + // exists(API::CallNode setProperty | + // setProperty = + // API::moduleImport("@angular/core") + // .getMember("Renderer2") + // .getInstance() + // .getMember("setProperty") + // .getACall() and + // this = setProperty.getParameter(2).asSink() and + // setProperty.getArgument(1).getStringValue() = "innerHTML" + // ) + // } + // } /** * A value being piped into the `safe` pipe in a template file, From 2dc9e7bab78a67b0eb9e9129a7a5b05edb9c72e6 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Wed, 8 Jan 2025 16:36:10 +0000 Subject: [PATCH 13/14] Moved def from AngularJSCore to Angular2 --- .../semmle/javascript/frameworks/Angular2.qll | 32 +++++++++++++++++++ .../frameworks/AngularJS/AngularJSCore.qll | 32 ------------------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Angular2.qll b/javascript/ql/lib/semmle/javascript/frameworks/Angular2.qll index 16430ff0475a..ba0f339f594e 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Angular2.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Angular2.qll @@ -554,4 +554,36 @@ module Angular2 { this = API::Node::ofType("@angular/core", "ElementRef").getMember("nativeElement").asSource() } } + + /** + * A DOM attribute write, using the AngularJS Renderer2 API: a call to `Renderer2.setProperty`. + */ + class AngularRenderer2AttributeDefinition extends DOM::AttributeDefinition { + DataFlow::Node propertyNode; + DataFlow::Node valueNode; + DataFlow::Node elementNode; + + AngularRenderer2AttributeDefinition() { + exists(API::CallNode setProperty | + setProperty = + API::moduleImport("@angular/core") + .getMember("Renderer2") + .getInstance() + .getMember("setProperty") + .getACall() and + elementNode = setProperty.getArgument(0) and + propertyNode = setProperty.getArgument(1) and + valueNode = setProperty.getArgument(2) and + this = setProperty.asExpr() + ) + } + + override string getName() { result = propertyNode.getStringValue() } + + // override DOM::ElementDefinition getElement() { /* TODO */ } + DataFlow::Node getElementNode() { result = elementNode } + + override DataFlow::Node getValueNode() { result = valueNode } + //override predicate mayHaveTemplateValue() { /* TODO */ } + } } diff --git a/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll b/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll index 2b4826c1529b..1a6d11cd7534 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll @@ -1032,35 +1032,3 @@ private class BindCall extends DataFlow::PartialInvokeNode::Range, DataFlow::Cal result = this.getArgument(0) } } - -/** - * A DOM attribute write, using the AngularJS Renderer2 API: a call to `Renderer2.setProperty`. - */ -private class AngularRenderer2AttributeDefinition extends DOM::AttributeDefinition { - DataFlow::Node propertyNode; - DataFlow::Node valueNode; - DataFlow::Node elementNode; - - AngularRenderer2AttributeDefinition() { - exists(API::CallNode setProperty | - setProperty = - API::moduleImport("@angular/core") - .getMember("Renderer2") - .getInstance() - .getMember("setProperty") - .getACall() and - elementNode = setProperty.getArgument(0) and - propertyNode = setProperty.getArgument(1) and - valueNode = setProperty.getArgument(2) and - this = setProperty.asExpr() - ) - } - - override string getName() { result = propertyNode.getStringValue() } - - // override DOM::ElementDefinition getElement() { /* TODO */ } - DataFlow::Node getElementNode() { result = elementNode } - - override DataFlow::Node getValueNode() { result = valueNode } - //override predicate mayHaveTemplateValue() { /* TODO */ } -} From 4b57d5feb2a346e79a71e8e6d60e3b66609f2509 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Wed, 8 Jan 2025 16:36:46 +0000 Subject: [PATCH 14/14] Added XSS sink for innerHTML/outerHTML using new Angular attribute def --- .../dataflow/DomBasedXssCustomizations.qll | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll index 026cf47106fd..e2a785ee4b14 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll @@ -251,25 +251,19 @@ module DomBasedXss { } } - // /** - // * A write to the `innerHTML` property of a DOM element, viewed as an XSS sink. - // * - // * Uses the Angular Renderer2 API, instead of the default `Element.innerHTML` property. - // */ - // class AngularRender2SetPropertyInnerHtmlSink extends Sink { - // AngularRender2SetPropertyInnerHtmlSink() { - // exists(API::CallNode setProperty | - // setProperty = - // API::moduleImport("@angular/core") - // .getMember("Renderer2") - // .getInstance() - // .getMember("setProperty") - // .getACall() and - // this = setProperty.getParameter(2).asSink() and - // setProperty.getArgument(1).getStringValue() = "innerHTML" - // ) - // } - // } + /** + * A write to the `innerHTML` or `outerHTML` property of a DOM element, viewed as an XSS sink. + * + * Uses the Angular Renderer2 API, instead of the default `Element.innerHTML` property. + */ + class AngularRender2SetPropertyInnerHtmlSink2 extends Sink { + AngularRender2SetPropertyInnerHtmlSink2() { + exists(Angular2::AngularRenderer2AttributeDefinition attrDef | + attrDef.getName() = ["innerHTML", "outerHTML"] and + this = attrDef.getValueNode() + ) + } + } /** * A value being piped into the `safe` pipe in a template file,