Skip to content

JS: remove cookie source; rely on persistent flow steps instead #698

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
semmle-qlci merged 3 commits into from
Dec 19, 2018
Merged

JS: remove cookie source; rely on persistent flow steps instead #698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
78334af
JS: remove cookie source; rely on persistent flow steps instead
asger-semmle Dec 13, 2018
60ae3e5
JS: update change note
asger-semmle Dec 19, 2018
ce18aca
JS: update expected output
asger-semmle Dec 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion change-notes/1.20/analysis-javascript.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## General improvements

* Support for popular libraries has been improved. Consequently, queries may produce more results on code bases that use the following features:
* Support for popular libraries has been improved. Consequently, queries may produce better results on code bases that use the following features:
- client-side code, for example [React](https://reactjs.org/)
- cookies and webstorage, for example [js-cookie](https://github.com/js-cookie/js-cookie)
- server-side code, for example [hapi](https://hapijs.com/)
Expand Down
14 changes: 0 additions & 14 deletions javascript/ql/src/semmle/javascript/security/dataflow/RemoteFlowSources.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,3 @@ abstract class RemoteFlowSource extends DataFlow::Node {
*/
predicate isUserControlledObject() { none() }
}

/**
* An access to `document.cookie`, viewed as a source of remote user input.
*/
private class DocumentCookieSource extends RemoteFlowSource, DataFlow::ValueNode {
DocumentCookieSource() {
isDocument(astNode.(PropAccess).getBase()) and
astNode.(PropAccess).getPropertyName() = "cookie"
}

override string getSourceType() {
result = "document.cookie"
}
}
10 changes: 8 additions & 2 deletions javascript/ql/test/query-tests/Security/CWE-022/TaintedPath.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ nodes
| TaintedPath.js:65:54:65:57 | path |
| TaintedPath.js:67:29:67:61 | pathMod ... h(path) |
| TaintedPath.js:67:57:67:60 | path |
| TaintedPath.js:78:26:78:40 | document.cookie |
| TaintedPath.js:78:26:78:45 | Cookie.get("unsafe") |
| TaintedPath.js:84:31:84:70 | require ... eq.url) |
| TaintedPath.js:84:31:84:76 | require ... ).query |
| TaintedPath.js:84:63:84:69 | req.url |
Expand All @@ -64,6 +64,9 @@ nodes
| TaintedPath.js:86:31:86:73 | require ... ).query |
| TaintedPath.js:86:60:86:66 | req.url |
| TaintedPath.js:94:48:94:60 | req.params[0] |
| TaintedPath.js:102:30:102:31 | ev |
| TaintedPath.js:103:24:103:25 | ev |
| TaintedPath.js:103:24:103:30 | ev.data |
| tainted-array-steps.js:9:7:9:48 | path |
| tainted-array-steps.js:9:14:9:37 | url.par ... , true) |
| tainted-array-steps.js:9:14:9:43 | url.par ... ).query |
Expand Down Expand Up @@ -143,6 +146,9 @@ edges
| TaintedPath.js:85:61:85:67 | req.url | TaintedPath.js:85:31:85:68 | require ... eq.url) |
| TaintedPath.js:86:31:86:67 | require ... eq.url) | TaintedPath.js:86:31:86:73 | require ... ).query |
| TaintedPath.js:86:60:86:66 | req.url | TaintedPath.js:86:31:86:67 | require ... eq.url) |
| TaintedPath.js:102:30:102:31 | ev | TaintedPath.js:103:24:103:25 | ev |
| TaintedPath.js:103:24:103:25 | ev | TaintedPath.js:103:24:103:30 | ev.data |
| TaintedPath.js:103:24:103:30 | ev.data | TaintedPath.js:78:26:78:45 | Cookie.get("unsafe") |
| tainted-array-steps.js:9:7:9:48 | path | tainted-array-steps.js:11:40:11:43 | path |
| tainted-array-steps.js:9:7:9:48 | path | tainted-array-steps.js:13:26:13:29 | path |
| tainted-array-steps.js:9:14:9:37 | url.par ... , true) | tainted-array-steps.js:9:14:9:43 | url.par ... ).query |
Expand Down Expand Up @@ -178,7 +184,7 @@ edges
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
Expand Down
6 changes: 5 additions & 1 deletion javascript/ql/test/query-tests/Security/CWE-022/TaintedPath.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ angular.module('myApp', [])
})
.directive('myCustomer', function() {
return {
templateUrl: document.cookie // NOT OK
templateUrl: Cookie.get("unsafe") // NOT OK
}
})

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

})();

addEventListener('message', (ev) => {
Cookie.set("unsafe", ev.data);
});
94 changes: 62 additions & 32 deletions javascript/ql/test/query-tests/Security/CWE-094/CodeInjection.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,32 @@
nodes
| angularjs.js:10:22:10:36 | document.cookie |
| angularjs.js:13:23:13:37 | document.cookie |
| angularjs.js:16:28:16:42 | document.cookie |
| angularjs.js:19:22:19:36 | document.cookie |
| angularjs.js:22:27:22:41 | document.cookie |
| angularjs.js:25:23:25:37 | document.cookie |
| angularjs.js:28:33:28:47 | document.cookie |
| angularjs.js:31:28:31:42 | document.cookie |
| angularjs.js:34:18:34:32 | document.cookie |
| angularjs.js:40:18:40:32 | document.cookie |
| angularjs.js:44:17:44:31 | document.cookie |
| angularjs.js:47:16:47:30 | document.cookie |
| angularjs.js:50:22:50:36 | document.cookie |
| angularjs.js:53:32:53:46 | document.cookie |
| angularjs.js:10:22:10:29 | location |
| angularjs.js:10:22:10:36 | location.search |
| angularjs.js:13:23:13:30 | location |
| angularjs.js:13:23:13:37 | location.search |
| angularjs.js:16:28:16:35 | location |
| angularjs.js:16:28:16:42 | location.search |
| angularjs.js:19:22:19:29 | location |
| angularjs.js:19:22:19:36 | location.search |
| angularjs.js:22:27:22:34 | location |
| angularjs.js:22:27:22:41 | location.search |
| angularjs.js:25:23:25:30 | location |
| angularjs.js:25:23:25:37 | location.search |
| angularjs.js:28:33:28:40 | location |
| angularjs.js:28:33:28:47 | location.search |
| angularjs.js:31:28:31:35 | location |
| angularjs.js:31:28:31:42 | location.search |
| angularjs.js:34:18:34:25 | location |
| angularjs.js:34:18:34:32 | location.search |
| angularjs.js:40:18:40:25 | location |
| angularjs.js:40:18:40:32 | location.search |
| angularjs.js:44:17:44:24 | location |
| angularjs.js:44:17:44:31 | location.search |
| angularjs.js:47:16:47:23 | location |
| angularjs.js:47:16:47:30 | location.search |
| angularjs.js:50:22:50:29 | location |
| angularjs.js:50:22:50:36 | location.search |
| angularjs.js:53:32:53:39 | location |
| angularjs.js:53:32:53:46 | location.search |
| eslint-escope-build.js:20:22:20:22 | c |
| eslint-escope-build.js:21:16:21:16 | c |
| express.js:7:24:7:62 | "return ... obble") |
Expand All @@ -33,13 +47,28 @@ nodes
| tst.js:2:6:2:83 | documen ... t=")+8) |
| tst.js:5:12:5:28 | document.location |
| tst.js:5:12:5:33 | documen ... on.hash |
| tst.js:14:10:14:24 | document.cookie |
| tst.js:14:10:14:65 | documen ... , "$1") |
| tst.js:14:10:14:26 | document.location |
| tst.js:14:10:14:33 | documen ... .search |
| tst.js:14:10:14:74 | documen ... , "$1") |
| tst.js:17:21:17:37 | document.location |
| tst.js:17:21:17:42 | documen ... on.hash |
| tst.js:20:30:20:46 | document.location |
| tst.js:20:30:20:51 | documen ... on.hash |
edges
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |
| angularjs.js:13:23:13:30 | location | angularjs.js:13:23:13:37 | location.search |
| angularjs.js:16:28:16:35 | location | angularjs.js:16:28:16:42 | location.search |
| angularjs.js:19:22:19:29 | location | angularjs.js:19:22:19:36 | location.search |
| angularjs.js:22:27:22:34 | location | angularjs.js:22:27:22:41 | location.search |
| angularjs.js:25:23:25:30 | location | angularjs.js:25:23:25:37 | location.search |
| angularjs.js:28:33:28:40 | location | angularjs.js:28:33:28:47 | location.search |
| angularjs.js:31:28:31:35 | location | angularjs.js:31:28:31:42 | location.search |
| angularjs.js:34:18:34:25 | location | angularjs.js:34:18:34:32 | location.search |
| angularjs.js:40:18:40:25 | location | angularjs.js:40:18:40:32 | location.search |
| angularjs.js:44:17:44:24 | location | angularjs.js:44:17:44:31 | location.search |
| angularjs.js:47:16:47:23 | location | angularjs.js:47:16:47:30 | location.search |
| angularjs.js:50:22:50:29 | location | angularjs.js:50:22:50:36 | location.search |
| angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search |
| eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c |
| express.js:7:24:7:62 | "return ... obble") | express.js:7:24:7:69 | "return ... + "];" |
| express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:62 | "return ... obble") |
Expand All @@ -53,24 +82,25 @@ edges
| tst.js:2:6:2:22 | document.location | tst.js:2:6:2:27 | documen ... on.href |
| tst.js:2:6:2:27 | documen ... on.href | tst.js:2:6:2:83 | documen ... t=")+8) |
| tst.js:5:12:5:28 | document.location | tst.js:5:12:5:33 | documen ... on.hash |
| tst.js:14:10:14:24 | document.cookie | tst.js:14:10:14:65 | documen ... , "$1") |
| tst.js:14:10:14:26 | document.location | tst.js:14:10:14:33 | documen ... .search |
| tst.js:14:10:14:33 | documen ... .search | tst.js:14:10:14:74 | documen ... , "$1") |
| tst.js:17:21:17:37 | document.location | tst.js:17:21:17:42 | documen ... on.hash |
| tst.js:20:30:20:46 | document.location | tst.js:20:30:20:51 | documen ... on.hash |
#select
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
Expand All @@ -79,6 +109,6 @@ edges
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
Loading