Skip to content

Commit e194021

Browse files
author
Max Schaefer
authored
Merge pull request #629 from esben-semmle/js/persistent-read-taint
JS: add persistent storage taint steps
2 parents 969fe6e + 6d6379f commit e194021

23 files changed

+284
-0
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@
44

55
* Support for popular libraries has been improved. Consequently, queries may produce more results on code bases that use the following features:
66
- client-side code, for example [React](https://reactjs.org/)
7+
- cookies and webstorage, for example [js-cookie](https://github.com/js-cookie/js-cookie)
78
- server-side code, for example [hapi](https://hapijs.com/)
89
* File classification has been improved to recognize additional generated files, for example files from [HTML Tidy](html-tidy.org).
910

11+
* The taint tracking library now recognizes flow through persistent storage, this may give more results for the security queries.
12+
1013
## New queries
1114

1215
| **Query** | **Tags** | **Purpose** |

‎javascript/ql/src/javascript.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ import semmle.javascript.frameworks.Azure
6060
import semmle.javascript.frameworks.Babel
6161
import semmle.javascript.frameworks.ComposedFunctions
6262
import semmle.javascript.frameworks.ClientRequests
63+
import semmle.javascript.frameworks.CookieLibraries
6364
import semmle.javascript.frameworks.Credentials
6465
import semmle.javascript.frameworks.CryptoLibraries
6566
import semmle.javascript.frameworks.DigitalOcean

‎javascript/ql/src/semmle/javascript/Concepts.qll‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,27 @@ abstract class DatabaseAccess extends DataFlow::Node {
6565
/** Gets an argument to this database access that is interpreted as a query. */
6666
abstract DataFlow::Node getAQueryArgument();
6767
}
68+
69+
/**
70+
* A data flow node that reads persistent data.
71+
*/
72+
abstract class PersistentReadAccess extends DataFlow::Node {
73+
74+
/**
75+
* Gets a corresponding persistent write, if any.
76+
*/
77+
abstract PersistentWriteAccess getAWrite();
78+
79+
}
80+
81+
/**
82+
* A data flow node that writes persistent data.
83+
*/
84+
abstract class PersistentWriteAccess extends DataFlow::Node {
85+
86+
/**
87+
* Gets the data flow node corresponding to the written value.
88+
*/
89+
abstract DataFlow::Node getValue();
90+
91+
}

‎javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,24 @@ module TaintTracking {
232232
}
233233
}
234234

235+
/**
236+
* A taint propagating data flow edge through persistent storage.
237+
*/
238+
private class StorageTaintStep extends AdditionalTaintStep {
239+
240+
PersistentReadAccess read;
241+
242+
StorageTaintStep() {
243+
this = read
244+
}
245+
246+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
247+
pred = read.getAWrite().getValue() and
248+
succ = read
249+
}
250+
251+
}
252+
235253
/**
236254
* A taint propagating data flow edge caused by the builtin array functions.
237255
*/

‎javascript/ql/src/semmle/javascript/frameworks/CookieLibraries.qll‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
2+
/**
3+
* Provides classes for reasoning about cookies.
4+
*/
5+
6+
import javascript
7+
8+
/**
9+
* A model of the `js-cookie` library (https://github.com/js-cookie/js-cookie).
10+
*/
11+
private module JsCookie {
12+
/**
13+
* Gets a function call that invokes method `name` of the `js-cookie` library.
14+
*/
15+
DataFlow::CallNode libMemberCall(string name) {
16+
result = DataFlow::globalVarRef("Cookie").getAMemberCall(name) or
17+
result = DataFlow::globalVarRef("Cookie").getAMemberCall("noConflict").getAMemberCall(name) or
18+
result = DataFlow::moduleMember("js-cookie", name).getACall()
19+
}
20+
21+
class ReadAccess extends PersistentReadAccess, DataFlow::CallNode {
22+
ReadAccess() { this = libMemberCall("get") }
23+
24+
override PersistentWriteAccess getAWrite() {
25+
getArgument(0).mayHaveStringValue(result.(WriteAccess).getKey())
26+
}
27+
}
28+
29+
class WriteAccess extends PersistentWriteAccess, DataFlow::CallNode {
30+
WriteAccess() { this = libMemberCall("set") }
31+
32+
string getKey() { getArgument(0).mayHaveStringValue(result) }
33+
34+
override DataFlow::Node getValue() { result = getArgument(1) }
35+
}
36+
}
37+
38+
/**
39+
* A model of the `browser-cookies` library (https://github.com/voltace/browser-cookies).
40+
*/
41+
private module BrowserCookies {
42+
/**
43+
* Gets a function call that invokes method `name` of the `browser-cookies` library.
44+
*/
45+
DataFlow::CallNode libMemberCall(string name) {
46+
result = DataFlow::moduleMember("browser-cookies", name).getACall()
47+
}
48+
49+
class ReadAccess extends PersistentReadAccess, DataFlow::CallNode {
50+
ReadAccess() { this = libMemberCall("get") }
51+
52+
override PersistentWriteAccess getAWrite() {
53+
getArgument(0).mayHaveStringValue(result.(WriteAccess).getKey())
54+
}
55+
}
56+
57+
class WriteAccess extends PersistentWriteAccess, DataFlow::CallNode {
58+
WriteAccess() { this = libMemberCall("set") }
59+
60+
string getKey() { getArgument(0).mayHaveStringValue(result) }
61+
62+
override DataFlow::Node getValue() { result = getArgument(1) }
63+
}
64+
}
65+
66+
/**
67+
* A model of the `cookie` library (https://github.com/jshttp/cookie).
68+
*/
69+
private module LibCookie {
70+
/**
71+
* Gets a function call that invokes method `name` of the `cookie` library.
72+
*/
73+
DataFlow::CallNode libMemberCall(string name) {
74+
result = DataFlow::moduleMember("cookie", name).getACall()
75+
}
76+
77+
class ReadAccess extends PersistentReadAccess {
78+
string key;
79+
ReadAccess() { this = libMemberCall("parse").getAPropertyRead(key) }
80+
81+
override PersistentWriteAccess getAWrite() {
82+
key = result.(WriteAccess).getKey()
83+
}
84+
}
85+
86+
class WriteAccess extends PersistentWriteAccess, DataFlow::CallNode {
87+
WriteAccess() { this = libMemberCall("serialize") }
88+
89+
string getKey() { getArgument(0).mayHaveStringValue(result) }
90+
91+
override DataFlow::Node getValue() { result = getArgument(1) }
92+
}
93+
}

‎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
*/

‎javascript/ql/test/library-tests/frameworks/Concepts/PersistentReadAccess.expected‎

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') |

‎javascript/ql/test/library-tests/frameworks/Concepts/PersistentReadAccess.ql‎

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

‎javascript/ql/test/library-tests/frameworks/Concepts/PersistentReadAccess_getAWrite.expected‎

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) |

‎javascript/ql/test/library-tests/frameworks/Concepts/PersistentReadAccess_getAWrite.ql‎

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()

0 commit comments

Comments
 (0)