Skip to content

Commit 28b4a78

Browse files
author
Esben Sparre Andreasen
committed
JS: introduce DOM::PersistentWebStorage
1 parent 7fb7527 commit 28b4a78

File tree

11 files changed

+105
-0
lines changed

11 files changed

+105
-0
lines changed

javascript/ql/src/semmle/javascript/security/dataflow/DOM.qll

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,44 @@ class WebStorageWrite extends Expr {
167167
}
168168
}
169169

170+
/**
171+
* Persistent storage through web storage such as `localStorage` or `sessionStorage`.
172+
*/
173+
private module PersistentWebStorage {
174+
private DataFlow::SourceNode webStorage(string kind) {
175+
(kind = "localStorage" or kind = "sessionStorage") and
176+
result = DataFlow::globalVarRef(kind)
177+
}
178+
179+
/**
180+
* A read access.
181+
*/
182+
class ReadAccess extends PersistentReadAccess, DataFlow::CallNode {
183+
string kind;
184+
185+
ReadAccess() { this = webStorage(kind).getAMethodCall("getItem") }
186+
187+
override PersistentWriteAccess getAWrite() {
188+
getArgument(0).mayHaveStringValue(result.(WriteAccess).getKey()) and
189+
result.(WriteAccess).getKind() = kind
190+
}
191+
}
192+
193+
/**
194+
* A write access.
195+
*/
196+
class WriteAccess extends PersistentWriteAccess, DataFlow::CallNode {
197+
string kind;
198+
199+
WriteAccess() { this = webStorage(kind).getAMethodCall("setItem") }
200+
201+
string getKey() { getArgument(0).mayHaveStringValue(result) }
202+
203+
string getKind() { result = kind }
204+
205+
override DataFlow::Node getValue() { result = getArgument(1) }
206+
}
207+
}
170208
/**
171209
* An event handler that handles `postMessage` events.
172210
*/
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| persistence.js:3:5:3:33 | localSt ... prop1') |
2+
| persistence.js:6:5:6:35 | session ... prop2') |
3+
| persistence.js:10:5:10:33 | localSt ... prop4') |
4+
| persistence.js:13:5:13:35 | session ... prop5') |
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import javascript
2+
3+
from PersistentReadAccess read
4+
select read
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| persistence.js:3:5:3:33 | localSt ... prop1') | persistence.js:2:5:2:37 | localSt ... 1', v1) |
2+
| persistence.js:6:5:6:35 | session ... prop2') | persistence.js:5:5:5:39 | session ... 2', v2) |
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import javascript
2+
3+
from PersistentReadAccess read
4+
select read, read.getAWrite()
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| persistence.js:2:5:2:37 | localSt ... 1', v1) | persistence.js:2:35:2:36 | v1 |
2+
| persistence.js:5:5:5:39 | session ... 2', v2) | persistence.js:5:37:5:38 | v2 |
3+
| persistence.js:8:5:8:37 | localSt ... 3', v3) | persistence.js:8:35:8:36 | v3 |
4+
| persistence.js:12:5:12:37 | localSt ... 5', v5) | persistence.js:12:35:12:36 | v5 |
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import javascript
2+
3+
from PersistentWriteAccess write
4+
select write, write.getValue()
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
(function(){
2+
localStorage.setItem('prop1', v1);
3+
localStorage.getItem('prop1');
4+
5+
sessionStorage.setItem('prop2', v2);
6+
sessionStorage.getItem('prop2');
7+
8+
localStorage.setItem('prop3', v3);
9+
10+
localStorage.getItem('prop4');
11+
12+
localStorage.setItem('prop5', v5);
13+
sessionStorage.getItem('prop5');
14+
});

javascript/ql/test/query-tests/Security/CWE-079/StoredXss.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ nodes
5454
| react-native.js:7:17:7:33 | req.param("code") |
5555
| react-native.js:8:18:8:24 | tainted |
5656
| react-native.js:9:27:9:33 | tainted |
57+
| stored-xss.js:2:39:2:55 | document.location |
58+
| stored-xss.js:2:39:2:62 | documen ... .search |
59+
| stored-xss.js:3:35:3:51 | document.location |
60+
| stored-xss.js:3:35:3:58 | documen ... .search |
61+
| stored-xss.js:5:20:5:52 | session ... ssion') |
62+
| stored-xss.js:8:20:8:48 | localSt ... local') |
5763
| string-manipulations.js:3:16:3:32 | document.location |
5864
| string-manipulations.js:4:16:4:32 | document.location |
5965
| string-manipulations.js:4:16:4:37 | documen ... on.href |
@@ -262,6 +268,10 @@ edges
262268
| react-native.js:7:7:7:33 | tainted | react-native.js:8:18:8:24 | tainted |
263269
| react-native.js:7:7:7:33 | tainted | react-native.js:9:27:9:33 | tainted |
264270
| react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted |
271+
| stored-xss.js:2:39:2:55 | document.location | stored-xss.js:2:39:2:62 | documen ... .search |
272+
| stored-xss.js:2:39:2:62 | documen ... .search | stored-xss.js:5:20:5:52 | session ... ssion') |
273+
| stored-xss.js:3:35:3:51 | document.location | stored-xss.js:3:35:3:58 | documen ... .search |
274+
| stored-xss.js:3:35:3:58 | documen ... .search | stored-xss.js:8:20:8:48 | localSt ... local') |
265275
| string-manipulations.js:4:16:4:32 | document.location | string-manipulations.js:4:16:4:37 | documen ... on.href |
266276
| string-manipulations.js:5:16:5:32 | document.location | string-manipulations.js:5:16:5:37 | documen ... on.href |
267277
| string-manipulations.js:5:16:5:37 | documen ... on.href | string-manipulations.js:5:16:5:47 | documen ... lueOf() |

javascript/ql/test/query-tests/Security/CWE-079/Xss.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ nodes
1717
| react-native.js:7:17:7:33 | req.param("code") |
1818
| react-native.js:8:18:8:24 | tainted |
1919
| react-native.js:9:27:9:33 | tainted |
20+
| stored-xss.js:2:39:2:55 | document.location |
21+
| stored-xss.js:2:39:2:62 | documen ... .search |
22+
| stored-xss.js:3:35:3:51 | document.location |
23+
| stored-xss.js:3:35:3:58 | documen ... .search |
24+
| stored-xss.js:5:20:5:52 | session ... ssion') |
25+
| stored-xss.js:8:20:8:48 | localSt ... local') |
2026
| string-manipulations.js:3:16:3:32 | document.location |
2127
| string-manipulations.js:4:16:4:32 | document.location |
2228
| string-manipulations.js:4:16:4:37 | documen ... on.href |
@@ -177,6 +183,10 @@ edges
177183
| react-native.js:7:7:7:33 | tainted | react-native.js:8:18:8:24 | tainted |
178184
| react-native.js:7:7:7:33 | tainted | react-native.js:9:27:9:33 | tainted |
179185
| react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted |
186+
| stored-xss.js:2:39:2:55 | document.location | stored-xss.js:2:39:2:62 | documen ... .search |
187+
| stored-xss.js:2:39:2:62 | documen ... .search | stored-xss.js:5:20:5:52 | session ... ssion') |
188+
| stored-xss.js:3:35:3:51 | document.location | stored-xss.js:3:35:3:58 | documen ... .search |
189+
| stored-xss.js:3:35:3:58 | documen ... .search | stored-xss.js:8:20:8:48 | localSt ... local') |
180190
| string-manipulations.js:4:16:4:32 | document.location | string-manipulations.js:4:16:4:37 | documen ... on.href |
181191
| string-manipulations.js:5:16:5:32 | document.location | string-manipulations.js:5:16:5:37 | documen ... on.href |
182192
| string-manipulations.js:5:16:5:37 | documen ... on.href | string-manipulations.js:5:16:5:47 | documen ... lueOf() |
@@ -296,6 +306,8 @@ edges
296306
| nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | HTML injection vulnerability due to $@. | nodemailer.js:13:50:13:66 | req.query.message | user-provided value |
297307
| react-native.js:8:18:8:24 | tainted | react-native.js:7:17:7:33 | req.param("code") | react-native.js:8:18:8:24 | tainted | Cross-site scripting vulnerability due to $@. | react-native.js:7:17:7:33 | req.param("code") | user-provided value |
298308
| react-native.js:9:27:9:33 | tainted | react-native.js:7:17:7:33 | req.param("code") | react-native.js:9:27:9:33 | tainted | Cross-site scripting vulnerability due to $@. | react-native.js:7:17:7:33 | req.param("code") | user-provided value |
309+
| stored-xss.js:5:20:5:52 | session ... ssion') | stored-xss.js:2:39:2:55 | document.location | stored-xss.js:5:20:5:52 | session ... ssion') | Cross-site scripting vulnerability due to $@. | stored-xss.js:2:39:2:55 | document.location | user-provided value |
310+
| stored-xss.js:8:20:8:48 | localSt ... local') | stored-xss.js:3:35:3:51 | document.location | stored-xss.js:8:20:8:48 | localSt ... local') | Cross-site scripting vulnerability due to $@. | stored-xss.js:3:35:3:51 | document.location | user-provided value |
299311
| string-manipulations.js:3:16:3:32 | document.location | string-manipulations.js:3:16:3:32 | document.location | string-manipulations.js:3:16:3:32 | document.location | Cross-site scripting vulnerability due to $@. | string-manipulations.js:3:16:3:32 | document.location | user-provided value |
300312
| string-manipulations.js:4:16:4:37 | documen ... on.href | string-manipulations.js:4:16:4:32 | document.location | string-manipulations.js:4:16:4:37 | documen ... on.href | Cross-site scripting vulnerability due to $@. | string-manipulations.js:4:16:4:32 | document.location | user-provided value |
301313
| string-manipulations.js:5:16:5:47 | documen ... lueOf() | string-manipulations.js:5:16:5:32 | document.location | string-manipulations.js:5:16:5:47 | documen ... lueOf() | Cross-site scripting vulnerability due to $@. | string-manipulations.js:5:16:5:32 | document.location | user-provided value |

0 commit comments

Comments
 (0)