Skip to content

JS: also consider relative exports when finding library inputs #12189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erik-krogh merged 2 commits into from
Feb 27, 2023
Merged

JS: also consider relative exports when finding library inputs #12189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b7305fd
also consider relative exports when finding library inputs
erik-krogh Feb 14, 2023
bec8dc6
add explicit this
erik-krogh Feb 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions javascript/ql/lib/semmle/javascript/NPM.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,20 +198,20 @@ class PackageJson extends JsonObject {
/**
* Gets the main module of this package.
*/
Module getMainModule() {
result = min(Module m, int prio | m.getFile() = resolveMainModule(this, prio) | m order by prio)
}
Module getMainModule() { result = this.getExportedModule(".") }

/**
* Gets the module exported under the given relative path.
*
* The main module is considered exported under the path `"."`.
*/
Module getExportedModule(string relativePath) {
relativePath = "." and
result = this.getMainModule()
or
result.getFile() = MainModulePath::of(this, relativePath).resolve()
result =
min(Module m, int prio |
m.getFile() = resolveMainModule(this, prio, relativePath)
|
m order by prio
)
}

/**
Expand Down
11 changes: 5 additions & 6 deletions javascript/ql/lib/semmle/javascript/NodeModuleResolutionImpl.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ File loadAsFile(Require req, int rootPriority, int priority) {
*/
File loadAsDirectory(Require req, int rootPriority, int priority) {
exists(Folder dir | dir = req.getImportedPath().resolve(rootPriority) |
result = resolveMainModule(dir.(NpmPackage).getPackageJson(), priority) or
result = resolveMainModule(dir.(NpmPackage).getPackageJson(), priority, ".") or
result = tryExtensions(dir, "index", priority - (numberOfExtensions() + 1))
)
}
Expand Down Expand Up @@ -132,12 +132,10 @@ private File resolveMainPath(PackageJson pkg, string mainPath, int priority) {
/**
* Gets the main module described by `pkg` with the given `priority`.
*/
File resolveMainModule(PackageJson pkg, int priority) {
exists(int subPriority, string mainPath |
result = resolveMainPath(pkg, mainPath, subPriority) and
if mainPath = "." then subPriority = priority else priority = subPriority + 1000
)
File resolveMainModule(PackageJson pkg, int priority, string exportPath) {
result = resolveMainPath(pkg, exportPath, priority)
or
exportPath = "." and
exists(Folder folder, Folder child |
child = folder or
child = folder.getChildContainer(getASrcFolderName()) or
Expand All @@ -149,6 +147,7 @@ File resolveMainModule(PackageJson pkg, int priority) {
)
or
// if there is no main module, then we look for files that are explicitly included in the published package.
exportPath = "." and
exists(PathExpr file |
// `FilesPath` only exists if there is no main module for a given package.
file = FilesPath::of(pkg) and priority = 100 // fixing the priority, because there might be multiple files in the package.
Expand Down
4 changes: 3 additions & 1 deletion javascript/ql/lib/semmle/javascript/PackageExports.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,9 @@ private DataFlow::Node getAValueExportedByPackage() {
DataFlow::globalVarRef("define").getACall().getAnArgument() = factory.getALocalUse() and
func.getFile() =
min(int j, File f |
f = NodeModule::resolveMainModule(any(PackageJson pack | exists(pack.getPackageName())), j)
f =
NodeModule::resolveMainModule(any(PackageJson pack | exists(pack.getPackageName())), j,
".")
|
f order by j
)
Expand Down
9 changes: 9 additions & 0 deletions .../test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/UnsafeHtmlConstruction.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ nodes
| lib2/index.ts:1:28:1:28 | s |
| lib2/index.ts:2:29:2:29 | s |
| lib2/index.ts:2:29:2:29 | s |
| lib2/src/MyNode.ts:1:28:1:28 | s |
| lib2/src/MyNode.ts:1:28:1:28 | s |
| lib2/src/MyNode.ts:2:29:2:29 | s |
| lib2/src/MyNode.ts:2:29:2:29 | s |
| lib/src/MyNode.ts:1:28:1:28 | s |
| lib/src/MyNode.ts:1:28:1:28 | s |
| lib/src/MyNode.ts:2:29:2:29 | s |
Expand Down Expand Up @@ -108,6 +112,10 @@ edges
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
| lib2/src/MyNode.ts:1:28:1:28 | s | lib2/src/MyNode.ts:2:29:2:29 | s |
| lib2/src/MyNode.ts:1:28:1:28 | s | lib2/src/MyNode.ts:2:29:2:29 | s |
| lib2/src/MyNode.ts:1:28:1:28 | s | lib2/src/MyNode.ts:2:29:2:29 | s |
| lib2/src/MyNode.ts:1:28:1:28 | s | lib2/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 |
| lib/src/MyNode.ts:1:28:1:28 | s | 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 |
Expand Down Expand Up @@ -200,6 +208,7 @@ edges
| 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 | This HTML construction which depends on $@ might later allow $@. | jquery-plugin.js:11:34:11:40 | options | library input | jquery-plugin.js:12:20:12:53 | "<span> ... /span>" | cross-site scripting |
| jquery-plugin.js:14:31:14:35 | stuff | jquery-plugin.js:11:27:11:31 | stuff | jquery-plugin.js:14:31:14:35 | stuff | This HTML construction which depends on $@ might later allow $@. | jquery-plugin.js:11:27:11:31 | stuff | library input | jquery-plugin.js:14:20:14:47 | "<span> ... /span>" | cross-site scripting |
| lib2/index.ts:2:29:2:29 | s | lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s | This HTML construction which depends on $@ might later allow $@. | lib2/index.ts:1:28:1:28 | s | library input | lib2/index.ts:3:49:3:52 | html | cross-site scripting |
| lib2/src/MyNode.ts:2:29:2:29 | s | lib2/src/MyNode.ts:1:28:1:28 | s | lib2/src/MyNode.ts:2:29:2:29 | s | This HTML construction which depends on $@ might later allow $@. | lib2/src/MyNode.ts:1:28:1:28 | s | library input | lib2/src/MyNode.ts:3:49:3:52 | html | cross-site scripting |
| 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 | This HTML construction which depends on $@ might later allow $@. | lib/src/MyNode.ts:1:28:1:28 | s | library input | lib/src/MyNode.ts:3:49:3:52 | html | cross-site scripting |
| main.js:2:29:2:29 | s | main.js:1:55:1:55 | s | main.js:2:29:2:29 | s | This HTML construction which depends on $@ might later allow $@. | main.js:1:55:1:55 | s | library input | main.js:3:49:3:52 | html | cross-site scripting |
| main.js:7:49:7:49 | s | main.js:6:49:6:49 | s | main.js:7:49:7:49 | s | This XML parsing which depends on $@ might later allow $@. | main.js:6:49:6:49 | s | library input | main.js:8:48:8:66 | doc.documentElement | cross-site scripting |
Expand Down
2 changes: 1 addition & 1 deletion javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/lib2/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "my-unsafe-library",
"main": "./foobar.js",
"main": "./index.ts",
"exports": {
"./MyNode": {
"require": "./lib/MyNode.cjs",
Expand Down
2 changes: 1 addition & 1 deletion javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/lib2/src/MyNode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export function trivialXss(s: string) {
const html = "<span>" + s + "</span>"; // OK - this file is not recognized as a main file.
const html = "<span>" + s + "</span>"; // NOT OK - this file is not recognized as a main file.
document.querySelector("#html").innerHTML = html;
}