Skip to content

Commit 1e048d8

Browse files
codeql-cicodeql-ci
authored
Merge pull request #4609 from asgerf/js/destructuring-export
Approved by erik-krogh
2 parents 31ec798 + 32c5bc4 commit 1e048d8

File tree

10 files changed

+82
-35
lines changed

10 files changed

+82
-35
lines changed

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,40 @@ class ES2015Module extends Module {
4040
}
4141
}
4242

43+
/**
44+
* Holds if `mod` contains one or more named export declarations other than `default`.
45+
*/
46+
private predicate hasNamedExports(ES2015Module mod) {
47+
mod.getAnExport().(ExportNamedDeclaration).getASpecifier().getExportedName() != "default"
48+
or
49+
exists(mod.getAnExport().(ExportNamedDeclaration).getAnExportedDecl())
50+
or
51+
// Bulk re-exports only export named bindings (not "default")
52+
mod.getAnExport() instanceof BulkReExportDeclaration
53+
}
54+
55+
/**
56+
* Holds if this module contains a `default` export.
57+
*/
58+
private predicate hasDefaultExport(ES2015Module mod) {
59+
// export default foo;
60+
mod.getAnExport() instanceof ExportDefaultDeclaration
61+
or
62+
// export { foo as default };
63+
mod.getAnExport().(ExportNamedDeclaration).getASpecifier().getExportedName() = "default"
64+
}
65+
66+
/**
67+
* Holds if `mod` contains both named and `default` exports.
68+
*
69+
* This is used to determine whether a default-import of the module should be reinterpreted
70+
* as a namespace-import, to accomodate the non-standard behavior implemented by some compilers.
71+
*/
72+
private predicate hasBothNamedAndDefaultExports(ES2015Module mod) {
73+
hasNamedExports(mod) and
74+
hasDefaultExport(mod)
75+
}
76+
4377
/**
4478
* An import declaration.
4579
*
@@ -70,6 +104,10 @@ class ImportDeclaration extends Stmt, Import, @import_declaration {
70104
is instanceof ImportNamespaceSpecifier and
71105
count(getASpecifier()) = 1
72106
or
107+
// For compatibility with the non-standard implementation of default imports,
108+
// treat default imports as namespace imports in cases where it can't cause ambiguity
109+
// between named exports and the properties of a default-exported object.
110+
not hasBothNamedAndDefaultExports(getImportedModule()) and
73111
is.getImportedName() = "default"
74112
)
75113
or

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -205,15 +205,16 @@ private predicate isRequire(DataFlow::Node nd) {
205205
or
206206
isRequire(nd.getAPredecessor())
207207
or
208-
// `import { createRequire } from 'module';` support.
209-
// specialized to ES2015 modules to avoid recursion in the `DataFlow::moduleImport()` predicate.
210-
exists(ImportDeclaration imp | imp.getImportedPath().getValue() = "module" |
211-
nd =
212-
imp
213-
.getImportedModuleNode()
214-
.(DataFlow::SourceNode)
215-
.getAPropertyRead("createRequire")
216-
.getACall()
208+
// `import { createRequire } from 'module';`.
209+
// specialized to ES2015 modules to avoid recursion in the `DataFlow::moduleImport()` predicate and to avoid
210+
// negative recursion between `Import.getImportedModuleNode()` and `Import.getImportedModule()`.
211+
exists(ImportDeclaration imp, DataFlow::SourceNode baseObj |
212+
imp.getImportedPath().getValue() = "module"
213+
|
214+
baseObj =
215+
[DataFlow::destructuredModuleImportNode(imp),
216+
DataFlow::valueNode(imp.getASpecifier().(ImportNamespaceSpecifier))] and
217+
nd = baseObj.getAPropertyRead("createRequire").getACall()
217218
)
218219
}
219220

‎javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,12 @@ private class AnalyzedVariableExport extends AnalyzedPropertyWrite, DataFlow::Va
318318
override predicate writes(AbstractValue baseVal, string propName, DataFlow::AnalyzedNode source) {
319319
baseVal = TAbstractExportsObject(export.getEnclosingModule()) and
320320
propName = name and
321-
source = varDef.getSource().analyze()
321+
(
322+
source = varDef.getSource().analyze()
323+
or
324+
varDef.getTarget() instanceof DestructuringPattern and
325+
source = export.getSourceNode(propName)
326+
)
322327
}
323328

324329
override predicate writesValue(AbstractValue baseVal, string propName, AbstractValue val) {

‎javascript/ql/test/library-tests/InterProceduralFlow/mixedExports.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
let source = 'tainted';
2+
3+
export const x = source;
4+
5+
export default {
6+
x: 'safe'
7+
};

‎javascript/ql/test/library-tests/InterProceduralFlow/mixedExportsClient.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import defaultValue from './mixedExports';
2+
import { x } from './mixedExports';
3+
import * as ns from './mixedExports';
4+
5+
let sink1 = defaultValue.x; // OK
6+
let sink2 = x; // NOT OK
7+
let sink3 = ns.x; // NOT OK

‎javascript/ql/test/library-tests/InterProceduralFlow/tests.expected‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ dataFlow
2626
| global.js:2:15:2:24 | "tainted2" | global.js:10:13:10:22 | g(source2) |
2727
| global.js:5:22:5:35 | "also tainted" | global.js:9:13:9:22 | g(source1) |
2828
| global.js:5:22:5:35 | "also tainted" | global.js:10:13:10:22 | g(source2) |
29+
| mixedExports.js:1:14:1:22 | 'tainted' | mixedExportsClient.js:6:13:6:13 | x |
30+
| mixedExports.js:1:14:1:22 | 'tainted' | mixedExportsClient.js:7:13:7:16 | ns.x |
2931
| nodeJsLib.js:2:15:2:23 | "tainted" | esClient.js:7:13:7:18 | nj.foo |
3032
| nodeJsLib.js:2:15:2:23 | "tainted" | esClient.js:10:13:10:17 | njFoo |
3133
| nodeJsLib.js:2:15:2:23 | "tainted" | nodeJsClient.js:4:13:4:18 | nj.foo |
@@ -106,6 +108,8 @@ taintTracking
106108
| global.js:2:15:2:24 | "tainted2" | global.js:10:13:10:22 | g(source2) |
107109
| global.js:5:22:5:35 | "also tainted" | global.js:9:13:9:22 | g(source1) |
108110
| global.js:5:22:5:35 | "also tainted" | global.js:10:13:10:22 | g(source2) |
111+
| mixedExports.js:1:14:1:22 | 'tainted' | mixedExportsClient.js:6:13:6:13 | x |
112+
| mixedExports.js:1:14:1:22 | 'tainted' | mixedExportsClient.js:7:13:7:16 | ns.x |
109113
| nodeJsLib.js:1:15:1:23 | "tainted" | esClient.js:7:13:7:18 | nj.foo |
110114
| nodeJsLib.js:1:15:1:23 | "tainted" | esClient.js:10:13:10:17 | njFoo |
111115
| nodeJsLib.js:1:15:1:23 | "tainted" | nodeJsClient.js:4:13:4:18 | nj.foo |
@@ -209,6 +213,8 @@ germanFlow
209213
| global.js:2:15:2:24 | "tainted2" | global.js:10:13:10:22 | g(source2) |
210214
| global.js:5:22:5:35 | "also tainted" | global.js:9:13:9:22 | g(source1) |
211215
| global.js:5:22:5:35 | "also tainted" | global.js:10:13:10:22 | g(source2) |
216+
| mixedExports.js:1:14:1:22 | 'tainted' | mixedExportsClient.js:6:13:6:13 | x |
217+
| mixedExports.js:1:14:1:22 | 'tainted' | mixedExportsClient.js:7:13:7:16 | ns.x |
212218
| nodeJsLib.js:2:15:2:23 | "tainted" | esClient.js:7:13:7:18 | nj.foo |
213219
| nodeJsLib.js:2:15:2:23 | "tainted" | esClient.js:10:13:10:17 | njFoo |
214220
| nodeJsLib.js:2:15:2:23 | "tainted" | nodeJsClient.js:4:13:4:18 | nj.foo |

‎javascript/ql/test/library-tests/Portals/PortalEntry.expected‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -717,19 +717,13 @@
717717
| (parameter 0 (member log (member console (global)))) | src/m3/index.js:3:43:3:61 | m1("Hello, world!") | true |
718718
| (parameter 0 (member m (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:4:15:4:18 | "hi" | false |
719719
| (parameter 0 (member m (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:4:15:4:18 | "hi" | true |
720-
| (parameter 0 (member m (instance (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:15:4:18 | "hi" | false |
721720
| (parameter 0 (member m (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:2:5:2:8 | "hi" | false |
722721
| (parameter 0 (member m (return (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:4:15:4:18 | "hi" | false |
723-
| (parameter 0 (member m (return (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:15:4:18 | "hi" | false |
724-
| (parameter 0 (member m (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:2:5:2:8 | "hi" | false |
725722
| (parameter 0 (member readFileSync (root https://www.npmjs.com/package/fs))) | src/m5/index.js:5:49:5:49 | f | false |
726723
| (parameter 0 (member s (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:5:15:5:21 | "there" | false |
727724
| (parameter 0 (member s (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:5:15:5:21 | "there" | true |
728-
| (parameter 0 (member s (instance (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:15:5:21 | "there" | false |
729725
| (parameter 0 (member s (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:3:5:3:11 | "there" | false |
730726
| (parameter 0 (member s (return (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:5:15:5:21 | "there" | false |
731-
| (parameter 0 (member s (return (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:15:5:21 | "there" | false |
732-
| (parameter 0 (member s (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:3:5:3:11 | "there" | false |
733727
| (parameter 0 (parameter 0 (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:2:6:2:8 | foo | true |
734728
| (parameter 0 (parameter 0 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic))))))))) | src/cyclic/index.js:2:6:2:8 | foo | true |
735729
| (parameter 0 (parameter 0 (member f00 (member f00 (member f00 (member f00 (member f00 (return (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:2:6:2:8 | foo | true |
@@ -1020,8 +1014,6 @@
10201014
| (parameter 0 (parameter 0 (return (return (return (return (return (return (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:2:6:2:8 | foo | true |
10211015
| (parameter 0 (parameter 1 (member then (instance (member Promise (root https://www.npmjs.com/package/bluebird)))))) | src/bluebird/index.js:6:12:6:15 | null | true |
10221016
| (parameter 0 (root https://www.npmjs.com/package/m1)) | src/m3/index.js:3:46:3:60 | "Hello, world!" | false |
1023-
| (parameter 0 (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:4:7:4:10 | "me" | false |
1024-
| (parameter 0 (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:5:7:5:10 | "me" | false |
10251017
| (return (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:3:10:3:12 | foo | true |
10261018
| (return (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic))))))))) | src/cyclic/index.js:3:10:3:12 | foo | true |
10271019
| (return (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (return (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:3:10:3:12 | foo | true |

‎javascript/ql/test/library-tests/Portals/PortalExit.expected‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
| (instance (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:4:1:4:11 | new A("me") | true |
2121
| (instance (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:5:1:5:11 | new A("me") | false |
2222
| (instance (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:5:1:5:11 | new A("me") | true |
23-
| (instance (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:4:1:4:11 | new A("me") | false |
24-
| (instance (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:5:1:5:11 | new A("me") | false |
2523
| (member String (global)) | src/m5/index.js:5:26:5:31 | String | true |
2624
| (member console (global)) | src/m2/main.js:2:3:2:9 | console | true |
2725
| (member console (global)) | src/m2/main.js:12:5:12:11 | console | true |
@@ -34,20 +32,14 @@
3432
| (member log (member console (global))) | src/m3/index.js:3:31:3:41 | console.log | true |
3533
| (member m (instance (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:1:4:13 | new A("me").m | false |
3634
| (member m (instance (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:1:4:13 | new A("me").m | true |
37-
| (member m (instance (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:4:1:4:13 | new A("me").m | false |
3835
| (member m (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:2:1:2:3 | A.m | false |
3936
| (member m (return (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:1:4:13 | new A("me").m | false |
40-
| (member m (return (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:4:1:4:13 | new A("me").m | false |
41-
| (member m (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:2:1:2:3 | A.m | false |
4237
| (member name (instance (member default (root https://www.npmjs.com/package/m2)))) | src/m2/main.js:12:27:12:35 | this.name | true |
4338
| (member readFileSync (root https://www.npmjs.com/package/fs)) | src/m5/index.js:5:33:5:47 | fs.readFileSync | false |
4439
| (member s (instance (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:1:5:13 | new A("me").s | false |
4540
| (member s (instance (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:1:5:13 | new A("me").s | true |
46-
| (member s (instance (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:5:1:5:13 | new A("me").s | false |
4741
| (member s (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:3:1:3:3 | A.s | false |
4842
| (member s (return (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:1:5:13 | new A("me").s | false |
49-
| (member s (return (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:5:1:5:13 | new A("me").s | false |
50-
| (member s (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:3:1:3:3 | A.s | false |
5143
| (member x (parameter 0 (member foo (root https://www.npmjs.com/package/m2)))) | src/m2/main.js:2:15:2:17 | p.x | true |
5244
| (member y (member x (parameter 0 (member foo (root https://www.npmjs.com/package/m2))))) | src/m2/main.js:2:15:2:19 | p.x.y | true |
5345
| (parameter 0 (member Promise (root https://www.npmjs.com/package/bluebird))) | src/bluebird/index.js:1:18:1:21 | exec | true |
@@ -764,19 +756,13 @@
764756
| (return (member log (member console (global)))) | src/m3/index.js:3:31:3:62 | console ... rld!")) | true |
765757
| (return (member m (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:4:1:4:19 | new A("me").m("hi") | false |
766758
| (return (member m (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:4:1:4:19 | new A("me").m("hi") | true |
767-
| (return (member m (instance (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:1:4:19 | new A("me").m("hi") | false |
768759
| (return (member m (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:2:1:2:9 | A.m("hi") | false |
769760
| (return (member m (return (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:4:1:4:19 | new A("me").m("hi") | false |
770-
| (return (member m (return (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:1:4:19 | new A("me").m("hi") | false |
771-
| (return (member m (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:2:1:2:9 | A.m("hi") | false |
772761
| (return (member readFileSync (root https://www.npmjs.com/package/fs))) | src/m5/index.js:5:33:5:50 | fs.readFileSync(f) | false |
773762
| (return (member s (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:5:1:5:22 | new A(" ... there") | false |
774763
| (return (member s (instance (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:5:1:5:22 | new A(" ... there") | true |
775-
| (return (member s (instance (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:1:5:22 | new A(" ... there") | false |
776764
| (return (member s (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:3:1:3:12 | A.s("there") | false |
777765
| (return (member s (return (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:5:1:5:22 | new A(" ... there") | false |
778-
| (return (member s (return (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:1:5:22 | new A(" ... there") | false |
779-
| (return (member s (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:3:1:3:12 | A.s("there") | false |
780766
| (return (parameter 0 (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:2:3:2:9 | cb(foo) | true |
781767
| (return (parameter 0 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic))))))))) | src/cyclic/index.js:2:3:2:9 | cb(foo) | true |
782768
| (return (parameter 0 (member f00 (member f00 (member f00 (member f00 (member f00 (return (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:2:3:2:9 | cb(foo) | true |
@@ -1067,11 +1053,8 @@
10671053
| (return (parameter 0 (return (return (return (return (return (return (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:2:3:2:9 | cb(foo) | true |
10681054
| (return (parameter 1 (member then (instance (member Promise (root https://www.npmjs.com/package/bluebird)))))) | src/bluebird/index.js:6:3:6:16 | rejected(null) | true |
10691055
| (return (root https://www.npmjs.com/package/m1)) | src/m3/index.js:3:43:3:61 | m1("Hello, world!") | false |
1070-
| (return (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:4:1:4:11 | new A("me") | false |
1071-
| (return (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:5:1:5:11 | new A("me") | false |
10721056
| (root https://www.npmjs.com/package/base-64/base64.js) | src/m5/index.js:2:14:2:41 | require ... 64.js") | false |
10731057
| (root https://www.npmjs.com/package/fs) | src/m5/index.js:1:12:1:24 | require("fs") | false |
10741058
| (root https://www.npmjs.com/package/m1) | src/m3/index.js:1:10:1:22 | require("m1") | false |
10751059
| (root https://www.npmjs.com/package/m2) | src/m3/tst2.js:1:1:1:25 | import ... m "m2"; | false |
10761060
| (root https://www.npmjs.com/package/m2) | src/m3/tst3.js:1:1:1:19 | import A from "m2"; | false |
1077-
| (root https://www.npmjs.com/package/m2) | src/m3/tst3.js:1:8:1:8 | A | false |

‎javascript/ql/test/library-tests/TypeTracking/destructuring-export-client.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import { foo, bar } from './destructuring-export';
2+
3+
foo(); // track: destructuring-export-foo
4+
bar(); // track: destructuring-export-bar

‎javascript/ql/test/library-tests/TypeTracking/destructuring-export.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
export const {
2+
foo, // name: destructuring-export-foo
3+
bar, // name: destructuring-export-bar
4+
} = new Something();

0 commit comments

Comments
 (0)