Skip to content

Commit f8694a3

Browse files
asgerfasgerf
authored
Merge pull request #18397 from aegilops/angular-sources-sinks
JavaScript CodeQL library updates: new Angular sink(s)
2 parents bc50634 + cda4b6f commit f8694a3

File tree

13 files changed

+186
-60
lines changed

13 files changed

+186
-60
lines changed

Diff for: ‎javascript/ql/lib/change-notes/2025-01-03-angular-source-sink.md

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: majorAnalysis
3+
---
4+
* Added new XSS sink where `innerHTML` or `outerHTML` is assigned to with the Angular Renderer2 API, plus modeled this API as a general attribute setter

Diff for: ‎javascript/ql/lib/semmle/javascript/frameworks/Angular2.qll

+35
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,41 @@ module Angular2 {
556556
}
557557
}
558558

559+
/**
560+
* A DOM attribute write, using the AngularJS Renderer2 API: a call to `Renderer2.setProperty`.
561+
*/
562+
class AngularRenderer2AttributeDefinition extends DOM::AttributeDefinition {
563+
DataFlow::Node propertyNode;
564+
DataFlow::Node valueNode;
565+
DataFlow::Node elementNode;
566+
567+
AngularRenderer2AttributeDefinition() {
568+
exists(API::CallNode setProperty |
569+
setProperty =
570+
API::moduleImport("@angular/core")
571+
.getMember("Renderer2")
572+
.getInstance()
573+
.getMember("setProperty")
574+
.getACall() and
575+
elementNode = setProperty.getArgument(0) and
576+
propertyNode = setProperty.getArgument(1) and
577+
valueNode = setProperty.getArgument(2) and
578+
this = setProperty.asExpr()
579+
)
580+
}
581+
582+
override string getName() { result = propertyNode.getStringValue() }
583+
584+
/**
585+
* Get the `DataFlow::Node` that is affected by this Attribute Definition.
586+
*
587+
* Defined instead of defining `getElement()`, which requires returning a DOM element definition, `ElementDefinition`.
588+
*/
589+
DataFlow::Node getElementNode() { result = elementNode }
590+
591+
override DataFlow::Node getValueNode() { result = valueNode }
592+
}
593+
559594
/**
560595
* A source of DOM events originating from the `$event` variable in an event handler installed in an Angular template.
561596
*/

Diff for: ‎javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll

+14
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,20 @@ module DomBasedXss {
287287
}
288288
}
289289

290+
/**
291+
* A write to the `innerHTML` or `outerHTML` property of a DOM element, viewed as an XSS sink.
292+
*
293+
* Uses the Angular Renderer2 API, instead of the default `Element.innerHTML` property.
294+
*/
295+
class AngularRender2SetPropertyInnerHtmlSink2 extends Sink {
296+
AngularRender2SetPropertyInnerHtmlSink2() {
297+
exists(Angular2::AngularRenderer2AttributeDefinition attrDef |
298+
attrDef.getName() = ["innerHTML", "outerHTML"] and
299+
this = attrDef.getValueNode()
300+
)
301+
}
302+
}
303+
290304
/**
291305
* A value being piped into the `safe` pipe in a template file,
292306
* disabling subsequent HTML escaping.

Diff for: ‎javascript/ql/src/Security/CWE-079/ExceptionXss.expected

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
nodes
2+
| examples/ExceptionXssAjv.js:11:18:11:33 | ajv.errorsText() | semmle.label | ajv.errorsText() |
3+
edges
4+
subpaths
5+
#select
6+
| examples/ExceptionXssAjv.js:11:18:11:33 | ajv.errorsText() | examples/ExceptionXssAjv.js:11:18:11:33 | ajv.errorsText() | examples/ExceptionXssAjv.js:11:18:11:33 | ajv.errorsText() | $@ is reinterpreted as HTML without escaping meta-characters. | examples/ExceptionXssAjv.js:11:18:11:33 | ajv.errorsText() | JSON schema validation error |

Diff for: ‎javascript/ql/src/Security/CWE-079/ReflectedXss.expected

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
edges
2+
| examples/ReflectedXss.js:6:33:6:45 | req.params.id | examples/ReflectedXss.js:6:14:6:45 | "Unknow ... rams.id | provenance | |
3+
nodes
4+
| examples/ReflectedXss.js:6:14:6:45 | "Unknow ... rams.id | semmle.label | "Unknow ... rams.id |
5+
| examples/ReflectedXss.js:6:33:6:45 | req.params.id | semmle.label | req.params.id |
6+
subpaths
7+
#select
8+
| examples/ReflectedXss.js:6:14:6:45 | "Unknow ... rams.id | examples/ReflectedXss.js:6:33:6:45 | req.params.id | examples/ReflectedXss.js:6:14:6:45 | "Unknow ... rams.id | Cross-site scripting vulnerability due to a $@. | examples/ReflectedXss.js:6:33:6:45 | req.params.id | user-provided value |

Diff for: ‎javascript/ql/src/Security/CWE-079/StoredXss.expected

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
edges
2+
| examples/StoredXss.js:5:44:5:52 | fileNames | examples/StoredXss.js:7:9:7:17 | fileNames | provenance | |
3+
| examples/StoredXss.js:7:9:7:17 | fileNames | examples/StoredXss.js:7:27:7:34 | fileName | provenance | |
4+
| examples/StoredXss.js:7:9:7:17 | fileNames | examples/StoredXss.js:11:9:11:12 | list | provenance | |
5+
| examples/StoredXss.js:7:27:7:34 | fileName | examples/StoredXss.js:9:30:9:37 | fileName | provenance | |
6+
| examples/StoredXss.js:9:30:9:37 | fileName | examples/StoredXss.js:9:13:9:47 | list | provenance | |
7+
| examples/StoredXss.js:11:9:11:12 | list | examples/StoredXss.js:11:9:11:23 | list | provenance | |
8+
| examples/StoredXss.js:11:9:11:23 | list | examples/StoredXss.js:12:18:12:21 | list | provenance | |
9+
nodes
10+
| examples/StoredXss.js:5:44:5:52 | fileNames | semmle.label | fileNames |
11+
| examples/StoredXss.js:7:9:7:17 | fileNames | semmle.label | fileNames |
12+
| examples/StoredXss.js:7:27:7:34 | fileName | semmle.label | fileName |
13+
| examples/StoredXss.js:9:13:9:47 | list | semmle.label | list |
14+
| examples/StoredXss.js:9:30:9:37 | fileName | semmle.label | fileName |
15+
| examples/StoredXss.js:11:9:11:12 | list | semmle.label | list |
16+
| examples/StoredXss.js:11:9:11:23 | list | semmle.label | list |
17+
| examples/StoredXss.js:12:18:12:21 | list | semmle.label | list |
18+
subpaths
19+
| examples/StoredXss.js:7:9:7:17 | fileNames | examples/StoredXss.js:7:27:7:34 | fileName | examples/StoredXss.js:9:13:9:47 | list | examples/StoredXss.js:11:9:11:12 | list |
20+
#select
21+
| examples/StoredXss.js:12:18:12:21 | list | examples/StoredXss.js:5:44:5:52 | fileNames | examples/StoredXss.js:12:18:12:21 | list | Stored cross-site scripting vulnerability due to $@. | examples/StoredXss.js:5:44:5:52 | fileNames | stored value |

Diff for: ‎javascript/ql/src/Security/CWE-079/UnsafeHtmlConstruction.expected

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
nodes
2+
edges
3+
subpaths
4+
#select

Diff for: ‎javascript/ql/src/Security/CWE-079/UnsafeJQueryPlugin.expected

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
edges
2+
| examples/UnsafeJQueryPlugin.js:1:31:1:37 | options | examples/UnsafeJQueryPlugin.js:3:22:3:28 | options | provenance | |
3+
| examples/UnsafeJQueryPlugin.js:3:22:3:28 | options | examples/UnsafeJQueryPlugin.js:3:22:3:43 | options ... elector | provenance | |
4+
nodes
5+
| examples/UnsafeJQueryPlugin.js:1:31:1:37 | options | semmle.label | options |
6+
| examples/UnsafeJQueryPlugin.js:3:22:3:28 | options | semmle.label | options |
7+
| examples/UnsafeJQueryPlugin.js:3:22:3:43 | options ... elector | semmle.label | options ... elector |
8+
subpaths
9+
#select
10+
| examples/UnsafeJQueryPlugin.js:3:22:3:43 | options ... elector | examples/UnsafeJQueryPlugin.js:1:31:1:37 | options | examples/UnsafeJQueryPlugin.js:3:22:3:43 | options ... elector | Potential XSS vulnerability in the $@. | examples/UnsafeJQueryPlugin.js:1:22:6:1 | functio ... ext);\\n} | '$.fn.copyText' plugin |

Diff for: ‎javascript/ql/src/Security/CWE-079/Xss.expected

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
nodes
2+
edges
3+
subpaths
4+
#select

Diff for: ‎javascript/ql/src/Security/CWE-079/XssThroughDom.expected

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
edges
2+
| examples/XssThroughDom.js:2:9:2:44 | target | examples/XssThroughDom.js:3:7:3:12 | target | provenance | |
3+
| examples/XssThroughDom.js:2:18:2:44 | $(this) ... arget") | examples/XssThroughDom.js:2:9:2:44 | target | provenance | |
4+
nodes
5+
| examples/XssThroughDom.js:2:9:2:44 | target | semmle.label | target |
6+
| examples/XssThroughDom.js:2:18:2:44 | $(this) ... arget") | semmle.label | $(this) ... arget") |
7+
| examples/XssThroughDom.js:3:7:3:12 | target | semmle.label | target |
8+
subpaths
9+
#select
10+
| examples/XssThroughDom.js:3:7:3:12 | target | examples/XssThroughDom.js:2:18:2:44 | $(this) ... arget") | examples/XssThroughDom.js:3:7:3:12 | target | $@ is reinterpreted as HTML without escaping meta-characters. | examples/XssThroughDom.js:2:18:2:44 | $(this) ... arget") | DOM text |

0 commit comments

Comments
 (0)