hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-25 16:55:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

JS: Move AngularJSTemplateUrlSink to ClientSideUrlRedirection query

Browse Source 
This is not perfect but at least we can be consistent about keeping URLs-that-lead-to-xss in the same query
This commit is contained in:
Asger F
2024-08-16 14:36:44 +02:00
parent 699d3a0a0a
commit 9ee7599aeb
7 changed files with 52 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll
View File

@@ -237,6 +237,15 @@ module ClientSideUrlRedirect {
override predicate isXssSink() { any() }
}
/**
* A `templateUrl` member of an AngularJS directive.
*/
private class AngularJSTemplateUrlSink extends Sink {
AngularJSTemplateUrlSink() { this = any(AngularJS::CustomDirective d).getMember("templateUrl") }
override predicate isXssSink() { any() }
}
private class SinkFromModel extends Sink {
SinkFromModel() { this = ModelOutput::getASinkNode("url-redirection").asSink() }
}

7
javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll
View File

@@ -653,10 +653,11 @@ module TaintedPath {
}
/**
* A `templateUrl` member of an AngularJS directive.
* DEPRECATED. This is no longer seen as a path-injection sink. It is tentatively handled
* by the client-side URL redirection query for now.
*/
class AngularJSTemplateUrlSink extends Sink, DataFlow::ValueNode {
AngularJSTemplateUrlSink() { this = any(AngularJS::CustomDirective d).getMember("templateUrl") }
deprecated class AngularJSTemplateUrlSink extends DataFlow::ValueNode instanceof Sink {
AngularJSTemplateUrlSink() { none() }
}
/**

39
javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected
View File

@@ -514,11 +514,6 @@ nodes
| TaintedPath.js:60:57:60:60 | path |
| TaintedPath.js:60:57:60:60 | path |
| TaintedPath.js:60:57:60:60 | path |
| TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") |
| TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") |
| TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") |
| TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") |
| TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") |
| TaintedPath.js:77:31:77:70 | require ... eq.url) |
| TaintedPath.js:77:31:77:70 | require ... eq.url) |
| TaintedPath.js:77:31:77:70 | require ... eq.url) |
@@ -639,19 +634,6 @@ nodes
| TaintedPath.js:87:48:87:60 | req.params[0] |
| TaintedPath.js:87:48:87:60 | req.params[0] |
| TaintedPath.js:87:48:87:60 | req.params[0] |
| TaintedPath.js:95:30:95:31 | ev |
| TaintedPath.js:95:30:95:31 | ev |
| TaintedPath.js:95:30:95:31 | ev |
| TaintedPath.js:95:30:95:31 | ev |
| TaintedPath.js:95:30:95:31 | ev |
| TaintedPath.js:96:24:96:25 | ev |
| TaintedPath.js:96:24:96:25 | ev |
| TaintedPath.js:96:24:96:25 | ev |
| TaintedPath.js:96:24:96:25 | ev |
| TaintedPath.js:96:24:96:30 | ev.data |
| TaintedPath.js:96:24:96:30 | ev.data |
| TaintedPath.js:96:24:96:30 | ev.data |
| TaintedPath.js:96:24:96:30 | ev.data |
| TaintedPath.js:100:6:100:47 | path |
| TaintedPath.js:100:6:100:47 | path |
| TaintedPath.js:100:6:100:47 | path |
@@ -5373,26 +5355,6 @@ edges
| TaintedPath.js:79:60:79:66 | req.url | TaintedPath.js:79:31:79:67 | require ... eq.url) |
| TaintedPath.js:79:60:79:66 | req.url | TaintedPath.js:79:31:79:67 | require ... eq.url) |
| TaintedPath.js:87:48:87:60 | req.params[0] | TaintedPath.js:87:48:87:60 | req.params[0] |
| TaintedPath.js:95:30:95:31 | ev | TaintedPath.js:96:24:96:25 | ev |
| TaintedPath.js:95:30:95:31 | ev | TaintedPath.js:96:24:96:25 | ev |
| TaintedPath.js:95:30:95:31 | ev | TaintedPath.js:96:24:96:25 | ev |
| TaintedPath.js:95:30:95:31 | ev | TaintedPath.js:96:24:96:25 | ev |
| TaintedPath.js:95:30:95:31 | ev | TaintedPath.js:96:24:96:25 | ev |
| TaintedPath.js:95:30:95:31 | ev | TaintedPath.js:96:24:96:25 | ev |
| TaintedPath.js:95:30:95:31 | ev | TaintedPath.js:96:24:96:25 | ev |
| TaintedPath.js:95:30:95:31 | ev | TaintedPath.js:96:24:96:25 | ev |
| TaintedPath.js:96:24:96:25 | ev | TaintedPath.js:96:24:96:30 | ev.data |
| TaintedPath.js:96:24:96:25 | ev | TaintedPath.js:96:24:96:30 | ev.data |
| TaintedPath.js:96:24:96:25 | ev | TaintedPath.js:96:24:96:30 | ev.data |
| TaintedPath.js:96:24:96:25 | ev | TaintedPath.js:96:24:96:30 | ev.data |
| TaintedPath.js:96:24:96:30 | ev.data | TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") |
| TaintedPath.js:96:24:96:30 | ev.data | TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") |
| TaintedPath.js:96:24:96:30 | ev.data | TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") |
| TaintedPath.js:96:24:96:30 | ev.data | TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") |
| TaintedPath.js:96:24:96:30 | ev.data | TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") |
| TaintedPath.js:96:24:96:30 | ev.data | TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") |
| TaintedPath.js:96:24:96:30 | ev.data | TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") |
| TaintedPath.js:96:24:96:30 | ev.data | TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") |
| TaintedPath.js:100:6:100:47 | path | TaintedPath.js:102:44:102:47 | path |
| TaintedPath.js:100:6:100:47 | path | TaintedPath.js:102:44:102:47 | path |
| TaintedPath.js:100:6:100:47 | path | TaintedPath.js:102:44:102:47 | path |
@@ -10483,7 +10445,6 @@ edges
| TaintedPath.js:56:29:56:52 | pathMod ... e(path) | TaintedPath.js:38:20:38:26 | req.url | TaintedPath.js:56:29:56:52 | pathMod ... e(path) | This path depends on a $@. | TaintedPath.js:38:20:38:26 | req.url | user-provided value |
| TaintedPath.js:58:29:58:61 | pathMod ... ath, z) | TaintedPath.js:38:20:38:26 | req.url | TaintedPath.js:58:29:58:61 | pathMod ... ath, z) | This path depends on a $@. | TaintedPath.js:38:20:38:26 | req.url | user-provided value |
| TaintedPath.js:60:29:60:61 | pathMod ... h(path) | TaintedPath.js:38:20:38:26 | req.url | TaintedPath.js:60:29:60:61 | pathMod ... h(path) | This path depends on a $@. | TaintedPath.js:38:20:38:26 | req.url | user-provided value |
| TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") | TaintedPath.js:95:30:95:31 | ev | TaintedPath.js:71:26:71:45 | Cookie.get("unsafe") | This path depends on a $@. | TaintedPath.js:95:30:95:31 | ev | user-provided value |
| TaintedPath.js:77:31:77:76 | require ... ).query | TaintedPath.js:77:63:77:69 | req.url | TaintedPath.js:77:31:77:76 | require ... ).query | This path depends on a $@. | TaintedPath.js:77:63:77:69 | req.url | user-provided value |
| TaintedPath.js:78:31:78:74 | require ... ).query | TaintedPath.js:78:61:78:67 | req.url | TaintedPath.js:78:31:78:74 | require ... ).query | This path depends on a $@. | TaintedPath.js:78:61:78:67 | req.url | user-provided value |
| TaintedPath.js:79:31:79:73 | require ... ).query | TaintedPath.js:79:60:79:66 | req.url | TaintedPath.js:79:31:79:73 | require ... ).query | This path depends on a $@. | TaintedPath.js:79:60:79:66 | req.url | user-provided value |

2
javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.js
View File

@@ -68,7 +68,7 @@ angular.module('myApp', [])
})
.directive('myCustomer', function() {
return {
templateUrl: Cookie.get("unsafe") // NOT OK
templateUrl: Cookie.get("unsafe") // OK - (no longer flagged by this query)
}
})

12
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected
View File

@@ -86,6 +86,12 @@ nodes
| angular2-client.ts:44:44:44:76 | routeSn ... ('foo') |
| angular2-client.ts:44:44:44:76 | routeSn ... ('foo') |
| angular2-client.ts:44:44:44:76 | routeSn ... ('foo') |
| angular-tempate-url.js:9:26:9:45 | Cookie.get("unsafe") |
| angular-tempate-url.js:9:26:9:45 | Cookie.get("unsafe") |
| angular-tempate-url.js:13:30:13:31 | ev |
| angular-tempate-url.js:13:30:13:31 | ev |
| angular-tempate-url.js:14:26:14:27 | ev |
| angular-tempate-url.js:14:26:14:32 | ev.data |
| classnames.js:7:31:7:84 | `<span ... <span>` |
| classnames.js:7:31:7:84 | `<span ... <span>` |
| classnames.js:7:47:7:69 | classNa ... w.name) |
@@ -1275,6 +1281,11 @@ edges
| angular2-client.ts:38:44:38:58 | this.router.url | angular2-client.ts:38:44:38:58 | this.router.url |
| angular2-client.ts:40:45:40:59 | this.router.url | angular2-client.ts:40:45:40:59 | this.router.url |
| angular2-client.ts:44:44:44:76 | routeSn ... ('foo') | angular2-client.ts:44:44:44:76 | routeSn ... ('foo') |
| angular-tempate-url.js:13:30:13:31 | ev | angular-tempate-url.js:14:26:14:27 | ev |
| angular-tempate-url.js:13:30:13:31 | ev | angular-tempate-url.js:14:26:14:27 | ev |
| angular-tempate-url.js:14:26:14:27 | ev | angular-tempate-url.js:14:26:14:32 | ev.data |
| angular-tempate-url.js:14:26:14:32 | ev.data | angular-tempate-url.js:9:26:9:45 | Cookie.get("unsafe") |
| angular-tempate-url.js:14:26:14:32 | ev.data | angular-tempate-url.js:9:26:9:45 | Cookie.get("unsafe") |
| classnames.js:7:47:7:69 | classNa ... w.name) | classnames.js:7:31:7:84 | `<span ... <span>` |
| classnames.js:7:47:7:69 | classNa ... w.name) | classnames.js:7:31:7:84 | `<span ... <span>` |
| classnames.js:7:58:7:68 | window.name | classnames.js:7:47:7:69 | classNa ... w.name) |
@@ -2407,6 +2418,7 @@ edges
| angular2-client.ts:38:44:38:58 | this.router.url | angular2-client.ts:38:44:38:58 | this.router.url | angular2-client.ts:38:44:38:58 | this.router.url | Cross-site scripting vulnerability due to $@. | angular2-client.ts:38:44:38:58 | this.router.url | user-provided value |
| angular2-client.ts:40:45:40:59 | this.router.url | angular2-client.ts:40:45:40:59 | this.router.url | angular2-client.ts:40:45:40:59 | this.router.url | Cross-site scripting vulnerability due to $@. | angular2-client.ts:40:45:40:59 | this.router.url | user-provided value |
| angular2-client.ts:44:44:44:76 | routeSn ... ('foo') | angular2-client.ts:44:44:44:76 | routeSn ... ('foo') | angular2-client.ts:44:44:44:76 | routeSn ... ('foo') | Cross-site scripting vulnerability due to $@. | angular2-client.ts:44:44:44:76 | routeSn ... ('foo') | user-provided value |
| angular-tempate-url.js:9:26:9:45 | Cookie.get("unsafe") | angular-tempate-url.js:13:30:13:31 | ev | angular-tempate-url.js:9:26:9:45 | Cookie.get("unsafe") | Cross-site scripting vulnerability due to $@. | angular-tempate-url.js:13:30:13:31 | ev | user-provided value |
| classnames.js:7:31:7:84 | `<span ... <span>` | classnames.js:7:58:7:68 | window.name | classnames.js:7:31:7:84 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:7:58:7:68 | window.name | user-provided value |
| classnames.js:8:31:8:85 | `<span ... <span>` | classnames.js:8:59:8:69 | window.name | classnames.js:8:31:8:85 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:8:59:8:69 | window.name | user-provided value |
| classnames.js:9:31:9:85 | `<span ... <span>` | classnames.js:9:59:9:69 | window.name | classnames.js:9:31:9:85 | `<span ... <span>` | Cross-site scripting vulnerability due to $@. | classnames.js:9:59:9:69 | window.name | user-provided value |

11
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected
View File

@@ -86,6 +86,12 @@ nodes
| angular2-client.ts:44:44:44:76 | routeSn ... ('foo') |
| angular2-client.ts:44:44:44:76 | routeSn ... ('foo') |
| angular2-client.ts:44:44:44:76 | routeSn ... ('foo') |
| angular-tempate-url.js:9:26:9:45 | Cookie.get("unsafe") |
| angular-tempate-url.js:9:26:9:45 | Cookie.get("unsafe") |
| angular-tempate-url.js:13:30:13:31 | ev |
| angular-tempate-url.js:13:30:13:31 | ev |
| angular-tempate-url.js:14:26:14:27 | ev |
| angular-tempate-url.js:14:26:14:32 | ev.data |
| classnames.js:7:31:7:84 | `<span ... <span>` |
| classnames.js:7:31:7:84 | `<span ... <span>` |
| classnames.js:7:47:7:69 | classNa ... w.name) |
@@ -1325,6 +1331,11 @@ edges
| angular2-client.ts:38:44:38:58 | this.router.url | angular2-client.ts:38:44:38:58 | this.router.url |
| angular2-client.ts:40:45:40:59 | this.router.url | angular2-client.ts:40:45:40:59 | this.router.url |
| angular2-client.ts:44:44:44:76 | routeSn ... ('foo') | angular2-client.ts:44:44:44:76 | routeSn ... ('foo') |
| angular-tempate-url.js:13:30:13:31 | ev | angular-tempate-url.js:14:26:14:27 | ev |
| angular-tempate-url.js:13:30:13:31 | ev | angular-tempate-url.js:14:26:14:27 | ev |
| angular-tempate-url.js:14:26:14:27 | ev | angular-tempate-url.js:14:26:14:32 | ev.data |
| angular-tempate-url.js:14:26:14:32 | ev.data | angular-tempate-url.js:9:26:9:45 | Cookie.get("unsafe") |
| angular-tempate-url.js:14:26:14:32 | ev.data | angular-tempate-url.js:9:26:9:45 | Cookie.get("unsafe") |
| classnames.js:7:47:7:69 | classNa ... w.name) | classnames.js:7:31:7:84 | `<span ... <span>` |
| classnames.js:7:47:7:69 | classNa ... w.name) | classnames.js:7:31:7:84 | `<span ... <span>` |
| classnames.js:7:58:7:68 | window.name | classnames.js:7:47:7:69 | classNa ... w.name) |

15
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/angular-tempate-url.js vendored Normal file
View File

@@ -0,0 +1,15 @@
angular.module('myApp', [])
.directive('myCustomer', function() {
return {
templateUrl: "SAFE" // OK
}
})
.directive('myCustomer', function() {
return {
templateUrl: Cookie.get("unsafe") // NOT OK
}
});
addEventListener('message', (ev) => {
Cookie.set("unsafe", ev.data);
});