Skip to content

Commit 6cbe4ca

Browse files
committed
support toJS() by using plain property names instead of pseudoproperties.
1 parent b1f092f commit 6cbe4ca

3 files changed

Lines changed: 22 additions & 12 deletions

File tree

javascript/ql/src/semmle/javascript/frameworks/Immutable.qll

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ import javascript
88
* Provides classes implementing data-flow for Immutable.
99
*/
1010
private module Immutable {
11-
private import DataFlow::PseudoProperties
12-
1311
/**
1412
* An API entrypoint for the global `Immutable` variable.
1513
*/
@@ -40,33 +38,35 @@ private module Immutable {
4038
}
4139

4240
/**
43-
* Gets the immutable collection where `pred` has been stored using the pseudoproperty `prop`.
41+
* An instance of any immutable collection.
42+
*/
43+
API::Node immutableCollection() { result = immutableMap() }
44+
45+
/**
46+
* Gets the immutable collection where `pred` has been stored using the name `prop`.
4447
*/
4548
DataFlow::SourceNode storeStep(DataFlow::Node pred, string prop) {
46-
exists(DataFlow::CallNode call, string key |
47-
call = immutableImport().getMember("Map").getACall()
48-
|
49-
prop = mapValueKey(key) and
50-
pred = call.getOptionArgument(0, key) and
49+
exists(DataFlow::CallNode call | call = immutableImport().getMember("Map").getACall() |
50+
pred = call.getOptionArgument(0, prop) and
5151
result = call
5252
)
5353
or
5454
exists(DataFlow::CallNode call | call = immutableMap().getMember("set").getACall() |
55-
prop = mapValue(call.getArgument(0)) and
55+
call.getArgument(0).mayHaveStringValue(prop) and
5656
pred = call.getArgument(1) and
5757
result = call
5858
)
5959
}
6060

6161
/**
62-
* Gets the value that was stored in the immutable collection `pred` under the pseudoproperty `prop`.
62+
* Gets the value that was stored in the immutable collection `pred` under the name `prop`.
6363
*/
6464
DataFlow::Node loadStep(DataFlow::Node pred, string prop) {
6565
// map.get()
6666
exists(DataFlow::MethodCallNode call | call = immutableMap().getMember("get").getACall() |
67+
call.getArgument(0).mayHaveStringValue(prop) and
6768
pred = call.getReceiver() and
68-
result = call and
69-
prop = mapValue(call.getArgument(0))
69+
result = call
7070
)
7171
}
7272

@@ -79,6 +79,12 @@ private module Immutable {
7979
pred = call.getReceiver() and
8080
result = call
8181
)
82+
or
83+
// toJS() or any immutable collection converts it to a plain JavaScript object/array.
84+
exists(DataFlow::CallNode call | call = immutableCollection().getMember("toJS").getACall() |
85+
pred = call.getReceiver() and
86+
result = call
87+
)
8288
}
8389

8490
/**

javascript/ql/test/library-tests/frameworks/Immutable/immutable.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,6 @@ sink(map2.get("b")); // OK - but still flagged [INCONSISTENCY]
1515
const map3 = map2.set("d", source("d"));
1616
sink(map1.get("d")); // OK
1717
sink(map3.get("d")); // NOT OK
18+
19+
20+
sink(map3.toJS()["a"]); // NOT OK
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
| immutable.js:1:16:1:26 | source("a") | immutable.js:2:6:2:13 | obj["a"] |
22
| immutable.js:1:16:1:26 | source("a") | immutable.js:11:6:11:18 | map1.get("a") |
33
| immutable.js:1:16:1:26 | source("a") | immutable.js:12:6:12:18 | map2.get("a") |
4+
| immutable.js:1:16:1:26 | source("a") | immutable.js:20:6:20:21 | map3.toJS()["a"] |
45
| immutable.js:1:32:1:43 | source("b1") | immutable.js:8:6:8:18 | map1.get("b") |
56
| immutable.js:1:32:1:43 | source("b1") | immutable.js:13:6:13:18 | map2.get("b") |
67
| immutable.js:15:28:15:38 | source("d") | immutable.js:17:6:17:18 | map3.get("d") |

0 commit comments

Comments
 (0)