Skip to content

Commit 495a1fc

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request #698 from asger-semmle/remove-cookie-as-source
Approved by esben-semmle
2 parents b11b714 + ce18aca commit 495a1fc

File tree

7 files changed

+96
-70
lines changed

7 files changed

+96
-70
lines changed

‎change-notes/1.20/analysis-javascript.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
## General improvements
44

5-
* Support for popular libraries has been improved. Consequently, queries may produce more results on code bases that use the following features:
5+
* Support for popular libraries has been improved. Consequently, queries may produce better results on code bases that use the following features:
66
- client-side code, for example [React](https://reactjs.org/)
77
- cookies and webstorage, for example [js-cookie](https://github.com/js-cookie/js-cookie)
88
- server-side code, for example [hapi](https://hapijs.com/)

‎javascript/ql/src/semmle/javascript/security/dataflow/RemoteFlowSources.qll‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,3 @@ abstract class RemoteFlowSource extends DataFlow::Node {
1616
*/
1717
predicate isUserControlledObject() { none() }
1818
}
19-
20-
/**
21-
* An access to `document.cookie`, viewed as a source of remote user input.
22-
*/
23-
private class DocumentCookieSource extends RemoteFlowSource, DataFlow::ValueNode {
24-
DocumentCookieSource() {
25-
isDocument(astNode.(PropAccess).getBase()) and
26-
astNode.(PropAccess).getPropertyName() = "cookie"
27-
}
28-
29-
override string getSourceType() {
30-
result = "document.cookie"
31-
}
32-
}

‎javascript/ql/test/query-tests/Security/CWE-022/TaintedPath.expected‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ nodes
5353
| TaintedPath.js:65:54:65:57 | path |
5454
| TaintedPath.js:67:29:67:61 | pathMod ... h(path) |
5555
| TaintedPath.js:67:57:67:60 | path |
56-
| TaintedPath.js:78:26:78:40 | document.cookie |
56+
| TaintedPath.js:78:26:78:45 | Cookie.get("unsafe") |
5757
| TaintedPath.js:84:31:84:70 | require ... eq.url) |
5858
| TaintedPath.js:84:31:84:76 | require ... ).query |
5959
| TaintedPath.js:84:63:84:69 | req.url |
@@ -64,6 +64,9 @@ nodes
6464
| TaintedPath.js:86:31:86:73 | require ... ).query |
6565
| TaintedPath.js:86:60:86:66 | req.url |
6666
| TaintedPath.js:94:48:94:60 | req.params[0] |
67+
| TaintedPath.js:102:30:102:31 | ev |
68+
| TaintedPath.js:103:24:103:25 | ev |
69+
| TaintedPath.js:103:24:103:30 | ev.data |
6770
| tainted-array-steps.js:9:7:9:48 | path |
6871
| tainted-array-steps.js:9:14:9:37 | url.par ... , true) |
6972
| tainted-array-steps.js:9:14:9:43 | url.par ... ).query |
@@ -143,6 +146,9 @@ edges
143146
| TaintedPath.js:85:61:85:67 | req.url | TaintedPath.js:85:31:85:68 | require ... eq.url) |
144147
| TaintedPath.js:86:31:86:67 | require ... eq.url) | TaintedPath.js:86:31:86:73 | require ... ).query |
145148
| TaintedPath.js:86:60:86:66 | req.url | TaintedPath.js:86:31:86:67 | require ... eq.url) |
149+
| TaintedPath.js:102:30:102:31 | ev | TaintedPath.js:103:24:103:25 | ev |
150+
| TaintedPath.js:103:24:103:25 | ev | TaintedPath.js:103:24:103:30 | ev.data |
151+
| TaintedPath.js:103:24:103:30 | ev.data | TaintedPath.js:78:26:78:45 | Cookie.get("unsafe") |
146152
| tainted-array-steps.js:9:7:9:48 | path | tainted-array-steps.js:11:40:11:43 | path |
147153
| tainted-array-steps.js:9:7:9:48 | path | tainted-array-steps.js:13:26:13:29 | path |
148154
| tainted-array-steps.js:9:14:9:37 | url.par ... , true) | tainted-array-steps.js:9:14:9:43 | url.par ... ).query |
@@ -178,7 +184,7 @@ edges
178184
| TaintedPath.js:63:29:63:52 | pathMod ... e(path) | TaintedPath.js:45:20:45:26 | req.url | TaintedPath.js:63:29:63:52 | pathMod ... e(path) | This path depends on $@. | TaintedPath.js:45:20:45:26 | req.url | a user-provided value |
179185
| TaintedPath.js:65:29:65:61 | pathMod ... ath, z) | TaintedPath.js:45:20:45:26 | req.url | TaintedPath.js:65:29:65:61 | pathMod ... ath, z) | This path depends on $@. | TaintedPath.js:45:20:45:26 | req.url | a user-provided value |
180186
| TaintedPath.js:67:29:67:61 | pathMod ... h(path) | TaintedPath.js:45:20:45:26 | req.url | TaintedPath.js:67:29:67:61 | pathMod ... h(path) | This path depends on $@. | TaintedPath.js:45:20:45:26 | req.url | a user-provided value |
181-
| TaintedPath.js:78:26:78:40 | document.cookie | TaintedPath.js:78:26:78:40 | document.cookie | TaintedPath.js:78:26:78:40 | document.cookie | This path depends on $@. | TaintedPath.js:78:26:78:40 | document.cookie | a user-provided value |
187+
| TaintedPath.js:78:26:78:45 | Cookie.get("unsafe") | TaintedPath.js:102:30:102:31 | ev | TaintedPath.js:78:26:78:45 | Cookie.get("unsafe") | This path depends on $@. | TaintedPath.js:102:30:102:31 | ev | a user-provided value |
182188
| TaintedPath.js:84:31:84:76 | require ... ).query | TaintedPath.js:84:63:84:69 | req.url | TaintedPath.js:84:31:84:76 | require ... ).query | This path depends on $@. | TaintedPath.js:84:63:84:69 | req.url | a user-provided value |
183189
| TaintedPath.js:85:31:85:74 | require ... ).query | TaintedPath.js:85:61:85:67 | req.url | TaintedPath.js:85:31:85:74 | require ... ).query | This path depends on $@. | TaintedPath.js:85:61:85:67 | req.url | a user-provided value |
184190
| TaintedPath.js:86:31:86:73 | require ... ).query | TaintedPath.js:86:60:86:66 | req.url | TaintedPath.js:86:31:86:73 | require ... ).query | This path depends on $@. | TaintedPath.js:86:60:86:66 | req.url | a user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-022/TaintedPath.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ angular.module('myApp', [])
7575
})
7676
.directive('myCustomer', function() {
7777
return {
78-
templateUrl: document.cookie // NOT OK
78+
templateUrl: Cookie.get("unsafe") // NOT OK
7979
}
8080
})
8181

@@ -98,3 +98,7 @@ var server = http.createServer(function(req, res) {
9898
application.get('/views/*', views_imported);
9999

100100
})();
101+
102+
addEventListener('message', (ev) => {
103+
Cookie.set("unsafe", ev.data);
104+
});

‎javascript/ql/test/query-tests/Security/CWE-094/CodeInjection.expected‎

Lines changed: 62 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,32 @@
11
nodes
2-
| angularjs.js:10:22:10:36 | document.cookie |
3-
| angularjs.js:13:23:13:37 | document.cookie |
4-
| angularjs.js:16:28:16:42 | document.cookie |
5-
| angularjs.js:19:22:19:36 | document.cookie |
6-
| angularjs.js:22:27:22:41 | document.cookie |
7-
| angularjs.js:25:23:25:37 | document.cookie |
8-
| angularjs.js:28:33:28:47 | document.cookie |
9-
| angularjs.js:31:28:31:42 | document.cookie |
10-
| angularjs.js:34:18:34:32 | document.cookie |
11-
| angularjs.js:40:18:40:32 | document.cookie |
12-
| angularjs.js:44:17:44:31 | document.cookie |
13-
| angularjs.js:47:16:47:30 | document.cookie |
14-
| angularjs.js:50:22:50:36 | document.cookie |
15-
| angularjs.js:53:32:53:46 | document.cookie |
2+
| angularjs.js:10:22:10:29 | location |
3+
| angularjs.js:10:22:10:36 | location.search |
4+
| angularjs.js:13:23:13:30 | location |
5+
| angularjs.js:13:23:13:37 | location.search |
6+
| angularjs.js:16:28:16:35 | location |
7+
| angularjs.js:16:28:16:42 | location.search |
8+
| angularjs.js:19:22:19:29 | location |
9+
| angularjs.js:19:22:19:36 | location.search |
10+
| angularjs.js:22:27:22:34 | location |
11+
| angularjs.js:22:27:22:41 | location.search |
12+
| angularjs.js:25:23:25:30 | location |
13+
| angularjs.js:25:23:25:37 | location.search |
14+
| angularjs.js:28:33:28:40 | location |
15+
| angularjs.js:28:33:28:47 | location.search |
16+
| angularjs.js:31:28:31:35 | location |
17+
| angularjs.js:31:28:31:42 | location.search |
18+
| angularjs.js:34:18:34:25 | location |
19+
| angularjs.js:34:18:34:32 | location.search |
20+
| angularjs.js:40:18:40:25 | location |
21+
| angularjs.js:40:18:40:32 | location.search |
22+
| angularjs.js:44:17:44:24 | location |
23+
| angularjs.js:44:17:44:31 | location.search |
24+
| angularjs.js:47:16:47:23 | location |
25+
| angularjs.js:47:16:47:30 | location.search |
26+
| angularjs.js:50:22:50:29 | location |
27+
| angularjs.js:50:22:50:36 | location.search |
28+
| angularjs.js:53:32:53:39 | location |
29+
| angularjs.js:53:32:53:46 | location.search |
1630
| eslint-escope-build.js:20:22:20:22 | c |
1731
| eslint-escope-build.js:21:16:21:16 | c |
1832
| express.js:7:24:7:62 | "return ... obble") |
@@ -33,13 +47,28 @@ nodes
3347
| tst.js:2:6:2:83 | documen ... t=")+8) |
3448
| tst.js:5:12:5:28 | document.location |
3549
| tst.js:5:12:5:33 | documen ... on.hash |
36-
| tst.js:14:10:14:24 | document.cookie |
37-
| tst.js:14:10:14:65 | documen ... , "$1") |
50+
| tst.js:14:10:14:26 | document.location |
51+
| tst.js:14:10:14:33 | documen ... .search |
52+
| tst.js:14:10:14:74 | documen ... , "$1") |
3853
| tst.js:17:21:17:37 | document.location |
3954
| tst.js:17:21:17:42 | documen ... on.hash |
4055
| tst.js:20:30:20:46 | document.location |
4156
| tst.js:20:30:20:51 | documen ... on.hash |
4257
edges
58+
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |
59+
| angularjs.js:13:23:13:30 | location | angularjs.js:13:23:13:37 | location.search |
60+
| angularjs.js:16:28:16:35 | location | angularjs.js:16:28:16:42 | location.search |
61+
| angularjs.js:19:22:19:29 | location | angularjs.js:19:22:19:36 | location.search |
62+
| angularjs.js:22:27:22:34 | location | angularjs.js:22:27:22:41 | location.search |
63+
| angularjs.js:25:23:25:30 | location | angularjs.js:25:23:25:37 | location.search |
64+
| angularjs.js:28:33:28:40 | location | angularjs.js:28:33:28:47 | location.search |
65+
| angularjs.js:31:28:31:35 | location | angularjs.js:31:28:31:42 | location.search |
66+
| angularjs.js:34:18:34:25 | location | angularjs.js:34:18:34:32 | location.search |
67+
| angularjs.js:40:18:40:25 | location | angularjs.js:40:18:40:32 | location.search |
68+
| angularjs.js:44:17:44:24 | location | angularjs.js:44:17:44:31 | location.search |
69+
| angularjs.js:47:16:47:23 | location | angularjs.js:47:16:47:30 | location.search |
70+
| angularjs.js:50:22:50:29 | location | angularjs.js:50:22:50:36 | location.search |
71+
| angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search |
4372
| eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |
4473
| express.js:7:24:7:62 | "return ... obble") | express.js:7:24:7:69 | "return ... + "];" |
4574
| express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:62 | "return ... obble") |
@@ -53,24 +82,25 @@ edges
5382
| tst.js:2:6:2:22 | document.location | tst.js:2:6:2:27 | documen ... on.href |
5483
| tst.js:2:6:2:27 | documen ... on.href | tst.js:2:6:2:83 | documen ... t=")+8) |
5584
| tst.js:5:12:5:28 | document.location | tst.js:5:12:5:33 | documen ... on.hash |
56-
| tst.js:14:10:14:24 | document.cookie | tst.js:14:10:14:65 | documen ... , "$1") |
85+
| tst.js:14:10:14:26 | document.location | tst.js:14:10:14:33 | documen ... .search |
86+
| tst.js:14:10:14:33 | documen ... .search | tst.js:14:10:14:74 | documen ... , "$1") |
5787
| tst.js:17:21:17:37 | document.location | tst.js:17:21:17:42 | documen ... on.hash |
5888
| tst.js:20:30:20:46 | document.location | tst.js:20:30:20:51 | documen ... on.hash |
5989
#select
60-
| angularjs.js:10:22:10:36 | document.cookie | angularjs.js:10:22:10:36 | document.cookie | angularjs.js:10:22:10:36 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:10:22:10:36 | document.cookie | User-provided value |
61-
| angularjs.js:13:23:13:37 | document.cookie | angularjs.js:13:23:13:37 | document.cookie | angularjs.js:13:23:13:37 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:13:23:13:37 | document.cookie | User-provided value |
62-
| angularjs.js:16:28:16:42 | document.cookie | angularjs.js:16:28:16:42 | document.cookie | angularjs.js:16:28:16:42 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:16:28:16:42 | document.cookie | User-provided value |
63-
| angularjs.js:19:22:19:36 | document.cookie | angularjs.js:19:22:19:36 | document.cookie | angularjs.js:19:22:19:36 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:19:22:19:36 | document.cookie | User-provided value |
64-
| angularjs.js:22:27:22:41 | document.cookie | angularjs.js:22:27:22:41 | document.cookie | angularjs.js:22:27:22:41 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:22:27:22:41 | document.cookie | User-provided value |
65-
| angularjs.js:25:23:25:37 | document.cookie | angularjs.js:25:23:25:37 | document.cookie | angularjs.js:25:23:25:37 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:25:23:25:37 | document.cookie | User-provided value |
66-
| angularjs.js:28:33:28:47 | document.cookie | angularjs.js:28:33:28:47 | document.cookie | angularjs.js:28:33:28:47 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:28:33:28:47 | document.cookie | User-provided value |
67-
| angularjs.js:31:28:31:42 | document.cookie | angularjs.js:31:28:31:42 | document.cookie | angularjs.js:31:28:31:42 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:31:28:31:42 | document.cookie | User-provided value |
68-
| angularjs.js:34:18:34:32 | document.cookie | angularjs.js:34:18:34:32 | document.cookie | angularjs.js:34:18:34:32 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:34:18:34:32 | document.cookie | User-provided value |
69-
| angularjs.js:40:18:40:32 | document.cookie | angularjs.js:40:18:40:32 | document.cookie | angularjs.js:40:18:40:32 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:40:18:40:32 | document.cookie | User-provided value |
70-
| angularjs.js:44:17:44:31 | document.cookie | angularjs.js:44:17:44:31 | document.cookie | angularjs.js:44:17:44:31 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:44:17:44:31 | document.cookie | User-provided value |
71-
| angularjs.js:47:16:47:30 | document.cookie | angularjs.js:47:16:47:30 | document.cookie | angularjs.js:47:16:47:30 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:47:16:47:30 | document.cookie | User-provided value |
72-
| angularjs.js:50:22:50:36 | document.cookie | angularjs.js:50:22:50:36 | document.cookie | angularjs.js:50:22:50:36 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:50:22:50:36 | document.cookie | User-provided value |
73-
| angularjs.js:53:32:53:46 | document.cookie | angularjs.js:53:32:53:46 | document.cookie | angularjs.js:53:32:53:46 | document.cookie | $@ flows to here and is interpreted as code. | angularjs.js:53:32:53:46 | document.cookie | User-provided value |
90+
| angularjs.js:10:22:10:36 | location.search | angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:10:22:10:29 | location | User-provided value |
91+
| angularjs.js:13:23:13:37 | location.search | angularjs.js:13:23:13:30 | location | angularjs.js:13:23:13:37 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:13:23:13:30 | location | User-provided value |
92+
| angularjs.js:16:28:16:42 | location.search | angularjs.js:16:28:16:35 | location | angularjs.js:16:28:16:42 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:16:28:16:35 | location | User-provided value |
93+
| angularjs.js:19:22:19:36 | location.search | angularjs.js:19:22:19:29 | location | angularjs.js:19:22:19:36 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:19:22:19:29 | location | User-provided value |
94+
| angularjs.js:22:27:22:41 | location.search | angularjs.js:22:27:22:34 | location | angularjs.js:22:27:22:41 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:22:27:22:34 | location | User-provided value |
95+
| angularjs.js:25:23:25:37 | location.search | angularjs.js:25:23:25:30 | location | angularjs.js:25:23:25:37 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:25:23:25:30 | location | User-provided value |
96+
| angularjs.js:28:33:28:47 | location.search | angularjs.js:28:33:28:40 | location | angularjs.js:28:33:28:47 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:28:33:28:40 | location | User-provided value |
97+
| angularjs.js:31:28:31:42 | location.search | angularjs.js:31:28:31:35 | location | angularjs.js:31:28:31:42 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:31:28:31:35 | location | User-provided value |
98+
| angularjs.js:34:18:34:32 | location.search | angularjs.js:34:18:34:25 | location | angularjs.js:34:18:34:32 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:34:18:34:25 | location | User-provided value |
99+
| angularjs.js:40:18:40:32 | location.search | angularjs.js:40:18:40:25 | location | angularjs.js:40:18:40:32 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:40:18:40:25 | location | User-provided value |
100+
| angularjs.js:44:17:44:31 | location.search | angularjs.js:44:17:44:24 | location | angularjs.js:44:17:44:31 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:44:17:44:24 | location | User-provided value |
101+
| angularjs.js:47:16:47:30 | location.search | angularjs.js:47:16:47:23 | location | angularjs.js:47:16:47:30 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:47:16:47:23 | location | User-provided value |
102+
| angularjs.js:50:22:50:36 | location.search | angularjs.js:50:22:50:29 | location | angularjs.js:50:22:50:36 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:50:22:50:29 | location | User-provided value |
103+
| angularjs.js:53:32:53:46 | location.search | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:53:32:53:39 | location | User-provided value |
74104
| eslint-escope-build.js:21:16:21:16 | c | eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c | $@ flows to here and is interpreted as code. | eslint-escope-build.js:20:22:20:22 | c | User-provided value |
75105
| express.js:7:24:7:69 | "return ... + "];" | express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" | $@ flows to here and is interpreted as code. | express.js:7:44:7:62 | req.param("wobble") | User-provided value |
76106
| express.js:9:34:9:79 | "return ... + "];" | express.js:9:54:9:72 | req.param("wobble") | express.js:9:34:9:79 | "return ... + "];" | $@ flows to here and is interpreted as code. | express.js:9:54:9:72 | req.param("wobble") | User-provided value |
@@ -79,6 +109,6 @@ edges
79109
| react-native.js:10:23:10:29 | tainted | react-native.js:7:17:7:33 | req.param("code") | react-native.js:10:23:10:29 | tainted | $@ flows to here and is interpreted as code. | react-native.js:7:17:7:33 | req.param("code") | User-provided value |
80110
| tst.js:2:6:2:83 | documen ... t=")+8) | tst.js:2:6:2:22 | document.location | tst.js:2:6:2:83 | documen ... t=")+8) | $@ flows to here and is interpreted as code. | tst.js:2:6:2:22 | document.location | User-provided value |
81111
| tst.js:5:12:5:33 | documen ... on.hash | tst.js:5:12:5:28 | document.location | tst.js:5:12:5:33 | documen ... on.hash | $@ flows to here and is interpreted as code. | tst.js:5:12:5:28 | document.location | User-provided value |
82-
| tst.js:14:10:14:65 | documen ... , "$1") | tst.js:14:10:14:24 | document.cookie | tst.js:14:10:14:65 | documen ... , "$1") | $@ flows to here and is interpreted as code. | tst.js:14:10:14:24 | document.cookie | User-provided value |
112+
| tst.js:14:10:14:74 | documen ... , "$1") | tst.js:14:10:14:26 | document.location | tst.js:14:10:14:74 | documen ... , "$1") | $@ flows to here and is interpreted as code. | tst.js:14:10:14:26 | document.location | User-provided value |
83113
| tst.js:17:21:17:42 | documen ... on.hash | tst.js:17:21:17:37 | document.location | tst.js:17:21:17:42 | documen ... on.hash | $@ flows to here and is interpreted as code. | tst.js:17:21:17:37 | document.location | User-provided value |
84114
| tst.js:20:30:20:51 | documen ... on.hash | tst.js:20:30:20:46 | document.location | tst.js:20:30:20:51 | documen ... on.hash | $@ flows to here and is interpreted as code. | tst.js:20:30:20:46 | document.location | User-provided value |

0 commit comments

Comments
 (0)