Skip to content

Commit 8d2768b

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#5634 from erik-krogh/fileSource
Approved by asgerf
2 parents 701e815 + 595bded commit 8d2768b

File tree

8 files changed

+114
-3
lines changed

8 files changed

+114
-3
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class PackageJSON extends JSONObject {
5151
string getAFile() { result = getFiles().getElementStringValue(_) }
5252

5353
/** Gets the main module of this package. */
54-
string getMain() { result = getPropStringValue("main") }
54+
string getMain() { result = MainModulePath::of(this).getValue() }
5555

5656
/** Gets the path of a command defined for this package. */
5757
string getBin(string cmd) {

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

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ File tryExtensions(Folder dir, string basename, int priority) {
8080
)
8181
}
8282

83+
/**
84+
* Gets `name` without a file extension.
85+
* Or `name`, if `name` has no file extension.
86+
*/
87+
bindingset[name]
88+
private string getStem(string name) { result = name.regexpCapture("(.+?)(?:\\.([^.]+))?", 1) }
89+
8390
/**
8491
* Gets the main module described by `pkg` with the given `priority`.
8592
*/
@@ -90,9 +97,8 @@ File resolveMainModule(PackageJSON pkg, int priority) {
9097
result = tryExtensions(main.resolve(), "index", priority)
9198
or
9299
not exists(main.resolve()) and
93-
not exists(main.getExtension()) and
94100
exists(int n | n = main.getNumComponent() |
95-
result = tryExtensions(main.resolveUpTo(n - 1), main.getComponent(n - 1), priority)
101+
result = tryExtensions(main.resolveUpTo(n - 1), getStem(main.getComponent(n - 1)), priority)
96102
)
97103
)
98104
or
@@ -101,6 +107,27 @@ File resolveMainModule(PackageJSON pkg, int priority) {
101107
tryExtensions([folder, folder.getChildContainer(["src", "lib"])], "index",
102108
priority - prioritiesPerCandidate())
103109
)
110+
or
111+
// if there is no main module, then we look for files that are explicitly included in the published package.
112+
exists(PathExpr file |
113+
// `FilesPath` only exists if there is no main module for a given package.
114+
file = FilesPath::of(pkg) and priority = 100 // fixing the priority, because there might be multiple files in the package.
115+
|
116+
result = file.resolve()
117+
or
118+
result = min(int i, File f | f = tryExtensions(file.resolve(), "index", i) | f order by i)
119+
or
120+
// resolve "file.js" to e.g. "file.ts".
121+
not exists(file.resolve()) and
122+
exists(int n | n = file.getNumComponent() |
123+
result =
124+
min(int i, File res |
125+
res = tryExtensions(file.resolveUpTo(n - 1), getStem(file.getComponent(n - 1)), i)
126+
|
127+
res order by i
128+
)
129+
)
130+
)
104131
}
105132

106133
/**
@@ -126,3 +153,31 @@ class MainModulePath extends PathExpr, @json_string {
126153
module MainModulePath {
127154
MainModulePath of(PackageJSON pkg) { result.getPackageJSON() = pkg }
128155
}
156+
157+
/**
158+
* A JSON string in a `package.json` file specifying a file that should be included in the published package.
159+
* These files are often imported directly from a client when a "main" module is not specified.
160+
* For performance reasons this only exists if there is no "main" field in the `package.json` file.
161+
*/
162+
private class FilesPath extends PathExpr, @json_string {
163+
PackageJSON pkg;
164+
165+
FilesPath() {
166+
this = pkg.getPropValue("files").(JSONArray).getElementValue(_) and
167+
not exists(MainModulePath::of(pkg))
168+
}
169+
170+
/** Gets the `package.json` file in which this path occurs. */
171+
PackageJSON getPackageJSON() { result = pkg }
172+
173+
override string getValue() { result = this.(JSONString).getValue() }
174+
175+
override Folder getAdditionalSearchRoot(int priority) {
176+
priority = 0 and
177+
result = pkg.getFile().getParentContainer()
178+
}
179+
}
180+
181+
private module FilesPath {
182+
FilesPath of(PackageJSON pkg) { result.getPackageJSON() = pkg }
183+
}

‎javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction.expected

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,18 @@ nodes
246246
| lib/lib.js:482:40:482:43 | name |
247247
| lib/lib.js:483:30:483:33 | name |
248248
| lib/lib.js:483:30:483:33 | name |
249+
| lib/subLib2/compiled-file.ts:3:26:3:29 | name |
250+
| lib/subLib2/compiled-file.ts:3:26:3:29 | name |
251+
| lib/subLib2/compiled-file.ts:4:25:4:28 | name |
252+
| lib/subLib2/compiled-file.ts:4:25:4:28 | name |
253+
| lib/subLib2/special-file.js:3:28:3:31 | name |
254+
| lib/subLib2/special-file.js:3:28:3:31 | name |
255+
| lib/subLib2/special-file.js:4:22:4:25 | name |
256+
| lib/subLib2/special-file.js:4:22:4:25 | name |
257+
| lib/subLib3/my-file.ts:3:28:3:31 | name |
258+
| lib/subLib3/my-file.ts:3:28:3:31 | name |
259+
| lib/subLib3/my-file.ts:4:22:4:25 | name |
260+
| lib/subLib3/my-file.ts:4:22:4:25 | name |
249261
| lib/subLib/index.js:3:28:3:31 | name |
250262
| lib/subLib/index.js:3:28:3:31 | name |
251263
| lib/subLib/index.js:4:22:4:25 | name |
@@ -546,6 +558,18 @@ edges
546558
| lib/lib.js:482:40:482:43 | name | lib/lib.js:483:30:483:33 | name |
547559
| lib/lib.js:482:40:482:43 | name | lib/lib.js:483:30:483:33 | name |
548560
| lib/lib.js:482:40:482:43 | name | lib/lib.js:483:30:483:33 | name |
561+
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
562+
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
563+
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
564+
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
565+
| lib/subLib2/special-file.js:3:28:3:31 | name | lib/subLib2/special-file.js:4:22:4:25 | name |
566+
| lib/subLib2/special-file.js:3:28:3:31 | name | lib/subLib2/special-file.js:4:22:4:25 | name |
567+
| lib/subLib2/special-file.js:3:28:3:31 | name | lib/subLib2/special-file.js:4:22:4:25 | name |
568+
| lib/subLib2/special-file.js:3:28:3:31 | name | lib/subLib2/special-file.js:4:22:4:25 | name |
569+
| lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name |
570+
| lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name |
571+
| lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name |
572+
| lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name |
549573
| lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name |
550574
| lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name |
551575
| lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name |
@@ -624,5 +648,8 @@ edges
624648
| lib/lib.js:447:13:447:28 | "rm -rf " + name | lib/lib.js:446:20:446:23 | name | lib/lib.js:447:25:447:28 | name | $@ based on $@ is later used in $@. | lib/lib.js:447:13:447:28 | "rm -rf " + name | String concatenation | lib/lib.js:446:20:446:23 | name | library input | lib/lib.js:447:3:447:29 | asyncEx ... + name) | shell command |
625649
| lib/lib.js:478:27:478:46 | config.installedPath | lib/lib.js:477:33:477:38 | config | lib/lib.js:478:27:478:46 | config.installedPath | $@ based on $@ is later used in $@. | lib/lib.js:478:27:478:46 | config.installedPath | Path concatenation | lib/lib.js:477:33:477:38 | config | library input | lib/lib.js:479:12:479:20 | exec(cmd) | shell command |
626650
| lib/lib.js:483:13:483:33 | ' my na ... + name | lib/lib.js:482:40:482:43 | name | lib/lib.js:483:30:483:33 | name | $@ based on $@ is later used in $@. | lib/lib.js:483:13:483:33 | ' my na ... + name | String concatenation | lib/lib.js:482:40:482:43 | name | library input | lib/lib.js:485:2:485:20 | cp.exec(cmd + args) | shell command |
651+
| lib/subLib2/compiled-file.ts:4:13:4:28 | "rm -rf " + name | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name | $@ based on $@ is later used in $@. | lib/subLib2/compiled-file.ts:4:13:4:28 | "rm -rf " + name | String concatenation | lib/subLib2/compiled-file.ts:3:26:3:29 | name | library input | lib/subLib2/compiled-file.ts:4:5:4:29 | cp.exec ... + name) | shell command |
652+
| lib/subLib2/special-file.js:4:10:4:25 | "rm -rf " + name | lib/subLib2/special-file.js:3:28:3:31 | name | lib/subLib2/special-file.js:4:22:4:25 | name | $@ based on $@ is later used in $@. | lib/subLib2/special-file.js:4:10:4:25 | "rm -rf " + name | String concatenation | lib/subLib2/special-file.js:3:28:3:31 | name | library input | lib/subLib2/special-file.js:4:2:4:26 | cp.exec ... + name) | shell command |
653+
| lib/subLib3/my-file.ts:4:10:4:25 | "rm -rf " + name | lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name | $@ based on $@ is later used in $@. | lib/subLib3/my-file.ts:4:10:4:25 | "rm -rf " + name | String concatenation | lib/subLib3/my-file.ts:3:28:3:31 | name | library input | lib/subLib3/my-file.ts:4:2:4:26 | cp.exec ... + name) | shell command |
627654
| lib/subLib/index.js:4:10:4:25 | "rm -rf " + name | lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name | $@ based on $@ is later used in $@. | lib/subLib/index.js:4:10:4:25 | "rm -rf " + name | String concatenation | lib/subLib/index.js:3:28:3:31 | name | library input | lib/subLib/index.js:4:2:4:26 | cp.exec ... + name) | shell command |
628655
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | lib/subLib/index.js:7:32:7:35 | name | lib/subLib/index.js:8:22:8:25 | name | $@ based on $@ is later used in $@. | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | String concatenation | lib/subLib/index.js:7:32:7:35 | name | library input | lib/subLib/index.js:8:2:8:26 | cp.exec ... + name) | shell command |

‎javascript/ql/test/query-tests/Security/CWE-078/lib/subLib2/compiled-file.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
var cp = require("child_process")
2+
3+
export default function (name) {
4+
cp.exec("rm -rf " + name); // NOT OK - the "files" directory points to this file.
5+
}

‎javascript/ql/test/query-tests/Security/CWE-078/lib/subLib2/package.json

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"name": "my-sub-lib2",
3+
"version": "0.0.7",
4+
"files": [
5+
"special-file.js",
6+
"compiled-file.js",
7+
"compiled-file"
8+
]
9+
}

‎javascript/ql/test/query-tests/Security/CWE-078/lib/subLib2/special-file.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
var cp = require("child_process")
2+
3+
module.exports = function (name) {
4+
cp.exec("rm -rf " + name); // NOT OK - the "files" directory points to this file.
5+
};

‎javascript/ql/test/query-tests/Security/CWE-078/lib/subLib3/my-file.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
var cp = require("child_process")
2+
3+
module.exports = function (name) {
4+
cp.exec("rm -rf " + name); // NOT OK - functions exported as part of a submodule are also flagged.
5+
};

‎javascript/ql/test/query-tests/Security/CWE-078/lib/subLib3/package.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"name": "my-sub-lib",
3+
"version": "0.0.7",
4+
"main": "./my-file.js"
5+
}

0 commit comments

Comments
 (0)