JS: Make moduleImport() work for named imports#934
JS: Make moduleImport() work for named imports#934semmle-qlci merged 3 commits intogithub:masterfrom
Conversation
|
That looks refreshingly simple! It's unfortunate we still treat named imports and default imports differently (and the test failure looks like it might be related to that), but it's certainly a big step in the right direction. Modulo evaluation and remaining test failures, this lvgtm! |
|
Right, I had forgotten about the ambiguous interpretation of default imports. Generalized it to default imports and checked that the test results are indeed to be expected and reasonable. |
| | global.js:5:22:5:35 | "also tainted" | global.js:9:13:9:22 | g(source1) | | ||
| | global.js:5:22:5:35 | "also tainted" | global.js:10:13:10:22 | g(source2) | | ||
| | nodeJsLib.js:1:15:1:23 | "tainted" | esClient.js:7:13:7:18 | nj.foo | | ||
| | nodeJsLib.js:1:15:1:23 | "tainted" | esClient.js:10:13:10:17 | njFoo | |
There was a problem hiding this comment.
For reference here's nodeJsLib.js
let source1 = "tainted";
let source2 = "tainted";
module.exports = source1;
exports.foo = source2;The exported object itself is tainted, and we have a taint step through property reads, so there is a path step from source1 -> (exports object).foo. This PR models import {foo} from 'nodeJsLib' as a PropRead so that step is new.
It shouldn't matter in practice since module exports objects are never tainted.
|
Evaluation is uneventful. |
Fixes https://jira.semmle.com/browse/ODASA-6953, so QL like this
now matches code like this
We're low on workers so I will start a full evaluation after reviews.