From db4b6d620a557da9aabf10f73682f35434626334 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 24 May 2022 14:06:17 +0200 Subject: [PATCH 1/3] JS: Remove Buffer.from as sink for js/resource-exhaustion --- .../dataflow/ResourceExhaustionCustomizations.qll | 2 -- .../ResourceExhaustion.expected | 15 --------------- .../ResourceExhaustion/resource-exhaustion.js | 6 +++--- 3 files changed, 3 insertions(+), 20 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionCustomizations.qll index 695876ac90f..e87f3cb7c54 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionCustomizations.qll @@ -81,8 +81,6 @@ module ResourceExhaustion { exists(string name | invk = clazz.getAMemberCall(name) and ( - name = "from" and index = 2 // the length argument - or name = ["alloc", "allocUnsafe", "allocUnsafeSlow"] and index = 0 // the buffer size ) ) diff --git a/javascript/ql/test/query-tests/Security/CWE-770/ResourceExhaustion/ResourceExhaustion.expected b/javascript/ql/test/query-tests/Security/CWE-770/ResourceExhaustion/ResourceExhaustion.expected index ffbdfd3d5e8..19adec40d24 100644 --- a/javascript/ql/test/query-tests/Security/CWE-770/ResourceExhaustion/ResourceExhaustion.expected +++ b/javascript/ql/test/query-tests/Security/CWE-770/ResourceExhaustion/ResourceExhaustion.expected @@ -17,12 +17,6 @@ nodes | resource-exhaustion.js:6:7:6:21 | n | | resource-exhaustion.js:6:11:6:21 | parseInt(s) | | resource-exhaustion.js:6:20:6:20 | s | -| resource-exhaustion.js:11:21:11:21 | s | -| resource-exhaustion.js:11:21:11:21 | s | -| resource-exhaustion.js:12:21:12:21 | n | -| resource-exhaustion.js:12:21:12:21 | n | -| resource-exhaustion.js:13:21:13:21 | n | -| resource-exhaustion.js:13:21:13:21 | n | | resource-exhaustion.js:14:16:14:16 | n | | resource-exhaustion.js:14:16:14:16 | n | | resource-exhaustion.js:15:22:15:22 | n | @@ -71,8 +65,6 @@ edges | documentaion-examples/ResourceExhaustion_timeout.js:5:33:5:39 | req.url | documentaion-examples/ResourceExhaustion_timeout.js:5:23:5:46 | url.par ... , true) | | documentaion-examples/ResourceExhaustion_timeout.js:5:33:5:39 | req.url | documentaion-examples/ResourceExhaustion_timeout.js:5:23:5:46 | url.par ... , true) | | resource-exhaustion.js:5:7:5:42 | s | resource-exhaustion.js:6:20:6:20 | s | -| resource-exhaustion.js:5:7:5:42 | s | resource-exhaustion.js:11:21:11:21 | s | -| resource-exhaustion.js:5:7:5:42 | s | resource-exhaustion.js:11:21:11:21 | s | | resource-exhaustion.js:5:7:5:42 | s | resource-exhaustion.js:35:12:35:12 | s | | resource-exhaustion.js:5:7:5:42 | s | resource-exhaustion.js:35:12:35:12 | s | | resource-exhaustion.js:5:7:5:42 | s | resource-exhaustion.js:82:17:82:17 | s | @@ -84,10 +76,6 @@ edges | resource-exhaustion.js:5:11:5:42 | url.par ... query.s | resource-exhaustion.js:5:7:5:42 | s | | resource-exhaustion.js:5:21:5:27 | req.url | resource-exhaustion.js:5:11:5:34 | url.par ... , true) | | resource-exhaustion.js:5:21:5:27 | req.url | resource-exhaustion.js:5:11:5:34 | url.par ... , true) | -| resource-exhaustion.js:6:7:6:21 | n | resource-exhaustion.js:12:21:12:21 | n | -| resource-exhaustion.js:6:7:6:21 | n | resource-exhaustion.js:12:21:12:21 | n | -| resource-exhaustion.js:6:7:6:21 | n | resource-exhaustion.js:13:21:13:21 | n | -| resource-exhaustion.js:6:7:6:21 | n | resource-exhaustion.js:13:21:13:21 | n | | resource-exhaustion.js:6:7:6:21 | n | resource-exhaustion.js:14:16:14:16 | n | | resource-exhaustion.js:6:7:6:21 | n | resource-exhaustion.js:14:16:14:16 | n | | resource-exhaustion.js:6:7:6:21 | n | resource-exhaustion.js:15:22:15:22 | n | @@ -124,9 +112,6 @@ edges | resource-exhaustion.js:6:20:6:20 | s | resource-exhaustion.js:6:11:6:21 | parseInt(s) | #select | documentaion-examples/ResourceExhaustion_timeout.js:7:16:7:20 | delay | documentaion-examples/ResourceExhaustion_timeout.js:5:33:5:39 | req.url | documentaion-examples/ResourceExhaustion_timeout.js:7:16:7:20 | delay | This creates a timer with a user-controlled duration from $@. | documentaion-examples/ResourceExhaustion_timeout.js:5:33:5:39 | req.url | here | -| resource-exhaustion.js:11:21:11:21 | s | resource-exhaustion.js:5:21:5:27 | req.url | resource-exhaustion.js:11:21:11:21 | s | This creates a buffer with a user-controlled size from $@. | resource-exhaustion.js:5:21:5:27 | req.url | here | -| resource-exhaustion.js:12:21:12:21 | n | resource-exhaustion.js:5:21:5:27 | req.url | resource-exhaustion.js:12:21:12:21 | n | This creates a buffer with a user-controlled size from $@. | resource-exhaustion.js:5:21:5:27 | req.url | here | -| resource-exhaustion.js:13:21:13:21 | n | resource-exhaustion.js:5:21:5:27 | req.url | resource-exhaustion.js:13:21:13:21 | n | This creates a buffer with a user-controlled size from $@. | resource-exhaustion.js:5:21:5:27 | req.url | here | | resource-exhaustion.js:14:16:14:16 | n | resource-exhaustion.js:5:21:5:27 | req.url | resource-exhaustion.js:14:16:14:16 | n | This creates a buffer with a user-controlled size from $@. | resource-exhaustion.js:5:21:5:27 | req.url | here | | resource-exhaustion.js:15:22:15:22 | n | resource-exhaustion.js:5:21:5:27 | req.url | resource-exhaustion.js:15:22:15:22 | n | This creates a buffer with a user-controlled size from $@. | resource-exhaustion.js:5:21:5:27 | req.url | here | | resource-exhaustion.js:16:26:16:26 | n | resource-exhaustion.js:5:21:5:27 | req.url | resource-exhaustion.js:16:26:16:26 | n | This creates a buffer with a user-controlled size from $@. | resource-exhaustion.js:5:21:5:27 | req.url | here | diff --git a/javascript/ql/test/query-tests/Security/CWE-770/ResourceExhaustion/resource-exhaustion.js b/javascript/ql/test/query-tests/Security/CWE-770/ResourceExhaustion/resource-exhaustion.js index e4b8f2ff9e3..ddda361db3a 100644 --- a/javascript/ql/test/query-tests/Security/CWE-770/ResourceExhaustion/resource-exhaustion.js +++ b/javascript/ql/test/query-tests/Security/CWE-770/ResourceExhaustion/resource-exhaustion.js @@ -8,9 +8,9 @@ var server = http.createServer(function(req, res) { Buffer.from(s); // OK Buffer.from(n); // OK Buffer.from(x, n); // OK - Buffer.from(x, y, s); // NOT OK - Buffer.from(x, y, n); // NOT OK - Buffer.from(x, y, n); // NOT OK + Buffer.from(x, y, s); // OK - does not allocate memory + Buffer.from(x, y, n); // OK - does not allocate memory + Buffer.from(x, y, n); // OK - does not allocate memory Buffer.alloc(n); // NOT OK Buffer.allocUnsafe(n); // NOT OK Buffer.allocUnsafeSlow(n); // NOT OK From 7d4a191a326249d4497f5083272b4c2756e97b12 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 24 May 2022 14:17:59 +0200 Subject: [PATCH 2/3] JS: Simplify --- .../dataflow/ResourceExhaustionCustomizations.qll | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionCustomizations.qll index e87f3cb7c54..8307c1f6f93 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionCustomizations.qll @@ -78,12 +78,8 @@ module ResourceExhaustion { exists(DataFlow::SourceNode clazz, DataFlow::InvokeNode invk, int index | clazz = DataFlow::globalVarRef("Buffer") and this = invk.getArgument(index) | - exists(string name | - invk = clazz.getAMemberCall(name) and - ( - name = ["alloc", "allocUnsafe", "allocUnsafeSlow"] and index = 0 // the buffer size - ) - ) + invk = clazz.getAMemberCall(["alloc", "allocUnsafe", "allocUnsafeSlow"]) and + index = 0 // the buffer size or invk = clazz.getAnInvocation() and ( From a955bd3695dfa4a5bd62c0b9a0983d9d1a7736c6 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 24 May 2022 14:16:41 +0200 Subject: [PATCH 3/3] JS: Change note --- .../2022-05-24-resource-exhaustion-no-buffer.from.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 javascript/ql/src/change-notes/2022-05-24-resource-exhaustion-no-buffer.from.md diff --git a/javascript/ql/src/change-notes/2022-05-24-resource-exhaustion-no-buffer.from.md b/javascript/ql/src/change-notes/2022-05-24-resource-exhaustion-no-buffer.from.md new file mode 100644 index 00000000000..8dadbdb4c93 --- /dev/null +++ b/javascript/ql/src/change-notes/2022-05-24-resource-exhaustion-no-buffer.from.md @@ -0,0 +1,5 @@ +--- +category: minorAnalysis +--- +* The `js/resource-exhaustion` query no longer treats the 3-argument version of `Buffer.from` as a sink, + since it does not allocate a new buffer.