Skip to content

Commit cb95e8f

Browse files
authored
Merge pull request #10351 from erik-krogh/moreMains
JS: find a main module in more cases
2 parents 7ca2e4c + 5010f89 commit cb95e8f

File tree

7 files changed

+74
-3
lines changed

7 files changed

+74
-3
lines changed

javascript/ql/lib/semmle/javascript/NodeModuleResolutionImpl.qll

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,11 @@ bindingset[name]
9090
private string getStem(string name) { result = name.regexpCapture("(.+?)(?:\\.([^.]+))?", 1) }
9191

9292
/**
93-
* Gets the main module described by `pkg` with the given `priority`.
93+
* Gets a file that a main module from `pkg` exported as `mainPath` with the given `priority`.
94+
* `mainPath` is "." if it's the main module of the package.
9495
*/
95-
File resolveMainModule(PackageJson pkg, int priority) {
96-
exists(PathExpr main | main = MainModulePath::of(pkg, ".") |
96+
private File resolveMainPath(PackageJson pkg, string mainPath, int priority) {
97+
exists(PathExpr main | main = MainModulePath::of(pkg, mainPath) |
9798
result = main.resolve() and priority = 0
9899
or
99100
result = tryExtensions(main.resolve(), "index", priority)
@@ -102,6 +103,26 @@ File resolveMainModule(PackageJson pkg, int priority) {
102103
exists(int n | n = main.getNumComponent() |
103104
result = tryExtensions(main.resolveUpTo(n - 1), getStem(main.getComponent(n - 1)), priority)
104105
)
106+
or
107+
// assuming the files get moved from one dir to another during compilation:
108+
not exists(main.resolve()) and // didn't resolve
109+
count(int i, string comp | comp = main.getComponent(i) and not comp = "." | i) = 2 and // is down one folder
110+
exists(Folder subFolder | subFolder = pkg.getFile().getParentContainer().getAFolder() |
111+
// is in one folder below the package.json, and has the right basename
112+
result =
113+
tryExtensions(subFolder, getStem(main.getComponent(main.getNumComponent() - 1)),
114+
priority - 999) // very high priority, to make sure everything else is tried first
115+
)
116+
)
117+
}
118+
119+
/**
120+
* Gets the main module described by `pkg` with the given `priority`.
121+
*/
122+
File resolveMainModule(PackageJson pkg, int priority) {
123+
exists(int subPriority, string mainPath |
124+
result = resolveMainPath(pkg, mainPath, subPriority) and
125+
if mainPath = "." then subPriority = priority else priority = subPriority + 1000
105126
)
106127
or
107128
exists(Folder folder, Folder child |

javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/UnsafeHtmlConstruction.expected

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@ nodes
88
| jquery-plugin.js:12:31:12:41 | options.foo |
99
| jquery-plugin.js:14:31:14:35 | stuff |
1010
| jquery-plugin.js:14:31:14:35 | stuff |
11+
| lib2/index.ts:1:28:1:28 | s |
12+
| lib2/index.ts:1:28:1:28 | s |
13+
| lib2/index.ts:2:29:2:29 | s |
14+
| lib2/index.ts:2:29:2:29 | s |
15+
| lib/src/MyNode.ts:1:28:1:28 | s |
16+
| lib/src/MyNode.ts:1:28:1:28 | s |
17+
| lib/src/MyNode.ts:2:29:2:29 | s |
18+
| lib/src/MyNode.ts:2:29:2:29 | s |
1119
| main.js:1:55:1:55 | s |
1220
| main.js:1:55:1:55 | s |
1321
| main.js:2:29:2:29 | s |
@@ -96,6 +104,14 @@ edges
96104
| jquery-plugin.js:11:34:11:40 | options | jquery-plugin.js:12:31:12:37 | options |
97105
| jquery-plugin.js:12:31:12:37 | options | jquery-plugin.js:12:31:12:41 | options.foo |
98106
| jquery-plugin.js:12:31:12:37 | options | jquery-plugin.js:12:31:12:41 | options.foo |
107+
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
108+
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
109+
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
110+
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
111+
| lib/src/MyNode.ts:1:28:1:28 | s | lib/src/MyNode.ts:2:29:2:29 | s |
112+
| lib/src/MyNode.ts:1:28:1:28 | s | lib/src/MyNode.ts:2:29:2:29 | s |
113+
| lib/src/MyNode.ts:1:28:1:28 | s | lib/src/MyNode.ts:2:29:2:29 | s |
114+
| lib/src/MyNode.ts:1:28:1:28 | s | lib/src/MyNode.ts:2:29:2:29 | s |
99115
| main.js:1:55:1:55 | s | main.js:2:29:2:29 | s |
100116
| main.js:1:55:1:55 | s | main.js:2:29:2:29 | s |
101117
| main.js:1:55:1:55 | s | main.js:2:29:2:29 | s |
@@ -183,6 +199,8 @@ edges
183199
#select
184200
| jquery-plugin.js:12:31:12:41 | options.foo | jquery-plugin.js:11:34:11:40 | options | jquery-plugin.js:12:31:12:41 | options.foo | $@ based on $@ might later cause $@. | jquery-plugin.js:12:31:12:41 | options.foo | HTML construction | jquery-plugin.js:11:34:11:40 | options | library input | jquery-plugin.js:12:20:12:53 | "<span> ... /span>" | cross-site scripting |
185201
| jquery-plugin.js:14:31:14:35 | stuff | jquery-plugin.js:11:27:11:31 | stuff | jquery-plugin.js:14:31:14:35 | stuff | $@ based on $@ might later cause $@. | jquery-plugin.js:14:31:14:35 | stuff | HTML construction | jquery-plugin.js:11:27:11:31 | stuff | library input | jquery-plugin.js:14:20:14:47 | "<span> ... /span>" | cross-site scripting |
202+
| lib2/index.ts:2:29:2:29 | s | lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s | $@ based on $@ might later cause $@. | lib2/index.ts:2:29:2:29 | s | HTML construction | lib2/index.ts:1:28:1:28 | s | library input | lib2/index.ts:3:49:3:52 | html | cross-site scripting |
203+
| lib/src/MyNode.ts:2:29:2:29 | s | lib/src/MyNode.ts:1:28:1:28 | s | lib/src/MyNode.ts:2:29:2:29 | s | $@ based on $@ might later cause $@. | lib/src/MyNode.ts:2:29:2:29 | s | HTML construction | lib/src/MyNode.ts:1:28:1:28 | s | library input | lib/src/MyNode.ts:3:49:3:52 | html | cross-site scripting |
186204
| main.js:2:29:2:29 | s | main.js:1:55:1:55 | s | main.js:2:29:2:29 | s | $@ based on $@ might later cause $@. | main.js:2:29:2:29 | s | HTML construction | main.js:1:55:1:55 | s | library input | main.js:3:49:3:52 | html | cross-site scripting |
187205
| main.js:7:49:7:49 | s | main.js:6:49:6:49 | s | main.js:7:49:7:49 | s | $@ based on $@ might later cause $@. | main.js:7:49:7:49 | s | XML parsing | main.js:6:49:6:49 | s | library input | main.js:8:48:8:66 | doc.documentElement | cross-site scripting |
188206
| main.js:12:49:12:49 | s | main.js:11:60:11:60 | s | main.js:12:49:12:49 | s | $@ based on $@ might later cause $@. | main.js:12:49:12:49 | s | XML parsing | main.js:11:60:11:60 | s | library input | main.js:16:21:16:35 | xml.cloneNode() | cross-site scripting |
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"name": "my-unsafe-library",
3+
"main": "./index.js",
4+
"exports": {
5+
"./MyNode": {
6+
"require": "./lib/MyNode.cjs",
7+
"import": "./lib/MyNode.mjs"
8+
}
9+
}
10+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
export function trivialXss(s: string) {
2+
const html = "<span>" + s + "</span>"; // NOT OK
3+
document.querySelector("#html").innerHTML = html;
4+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
export function trivialXss(s: string) {
2+
const html = "<span>" + s + "</span>"; // NOT OK - this file is recognized as a main file.
3+
document.querySelector("#html").innerHTML = html;
4+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"name": "my-unsafe-library",
3+
"main": "./foobar.js",
4+
"exports": {
5+
"./MyNode": {
6+
"require": "./lib/MyNode.cjs",
7+
"import": "./lib/MyNode.mjs"
8+
}
9+
}
10+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
export function trivialXss(s: string) {
2+
const html = "<span>" + s + "</span>"; // OK - this file is not recognized as a main file.
3+
document.querySelector("#html").innerHTML = html;
4+
}

0 commit comments

Comments
 (0)