Skip to content

Commit 8ff9c98

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#5449 from erik-krogh/asExec
Approved by esbena
2 parents 32dc894 + 36b0ab1 commit 8ff9c98

File tree

6 files changed

+121
-3
lines changed

6 files changed

+121
-3
lines changed

‎javascript/change-notes/2021-03-19-async-execute.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
lgtm,codescanning
2+
* The command injection security queries now recognize additional sinks.
3+
Affected packages are
4+
[async-execute](https://npmjs.com/package/async-execute)

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,58 @@ private DataFlow::Node getAValueExportedByPackage() {
7070
result = cla.getAStaticMethod() or
7171
result = cla.getConstructor()
7272
)
73+
or
74+
// *****
75+
// Common styles of transforming exported objects.
76+
// *****
77+
//
78+
// Object.defineProperties
79+
exists(DataFlow::MethodCallNode call |
80+
call = DataFlow::globalVarRef("Object").getAMethodCall("defineProperties") and
81+
[call, call.getArgument(0)] = getAValueExportedByPackage() and
82+
result = call.getArgument(any(int i | i > 0))
83+
)
84+
or
85+
// Object.defineProperty
86+
exists(CallToObjectDefineProperty call |
87+
[call, call.getBaseObject()] = getAValueExportedByPackage()
88+
|
89+
result = call.getPropertyDescriptor().getALocalSource().getAPropertyReference("value")
90+
or
91+
result =
92+
call.getPropertyDescriptor()
93+
.getALocalSource()
94+
.getAPropertyReference("get")
95+
.(DataFlow::FunctionNode)
96+
.getAReturn()
97+
)
98+
or
99+
// Object.assign and friends
100+
exists(ExtendCall assign |
101+
getAValueExportedByPackage() = [assign, assign.getDestinationOperand()] and
102+
result = assign.getASourceOperand()
103+
)
104+
or
105+
// Array.prototype.{map, reduce, entries, values}
106+
exists(DataFlow::MethodCallNode map |
107+
map.getMethodName() = ["map", "reduce", "entries", "values"] and
108+
map = getAValueExportedByPackage()
109+
|
110+
result = map.getArgument(0).getABoundFunctionValue(_).getAReturn()
111+
or
112+
// assuming that the receiver of the call is somehow exported
113+
result = map.getReceiver()
114+
)
115+
or
116+
// Object.{fromEntries, freeze, seal, entries, values}
117+
exists(DataFlow::MethodCallNode freeze |
118+
freeze =
119+
DataFlow::globalVarRef("Object")
120+
.getAMethodCall(["fromEntries", "freeze", "seal", "entries", "values"])
121+
|
122+
freeze = getAValueExportedByPackage() and
123+
result = freeze.getArgument(0)
124+
)
73125
}
74126

75127
/**

‎javascript/ql/src/semmle/javascript/dataflow/TypeTracking.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,12 @@ class TypeBackTracker extends TTypeBackTracker {
253253
*/
254254
predicate start() { hasReturn = false and prop = "" }
255255

256+
/**
257+
* Holds if this is the starting point of type backtracking, and the value is in the property named `propName`.
258+
* The type tracking only ends after the property has been stored.
259+
*/
260+
predicate isInProp(PropertyName propName) { hasReturn = false and prop = propName }
261+
256262
/**
257263
* Holds if this is the end point of type tracking.
258264
*/

‎javascript/ql/src/semmle/javascript/frameworks/SystemCommandExecutors.qll

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,15 @@ private predicate execApi(string mod, int cmdArg, int optionsArg, boolean shell)
4343
)
4444
or
4545
shell = true and
46-
mod = "exec" and
47-
optionsArg = -2 and
48-
cmdArg = 0
46+
(
47+
mod = "exec" and
48+
optionsArg = -2 and
49+
cmdArg = 0
50+
or
51+
mod = "async-execute" and
52+
optionsArg = 1 and
53+
cmdArg = 0
54+
)
4955
}
5056

5157
private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::InvokeNode {

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,14 @@ nodes
205205
| lib/lib.js:405:39:405:42 | name |
206206
| lib/lib.js:406:22:406:25 | name |
207207
| lib/lib.js:406:22:406:25 | name |
208+
| lib/lib.js:413:39:413:42 | name |
209+
| lib/lib.js:413:39:413:42 | name |
210+
| lib/lib.js:414:24:414:27 | name |
211+
| lib/lib.js:414:24:414:27 | name |
212+
| lib/lib.js:418:20:418:23 | name |
213+
| lib/lib.js:418:20:418:23 | name |
214+
| lib/lib.js:419:25:419:28 | name |
215+
| lib/lib.js:419:25:419:28 | name |
208216
edges
209217
| lib/lib2.js:3:28:3:31 | name | lib/lib2.js:4:22:4:25 | name |
210218
| lib/lib2.js:3:28:3:31 | name | lib/lib2.js:4:22:4:25 | name |
@@ -444,6 +452,14 @@ edges
444452
| lib/lib.js:405:39:405:42 | name | lib/lib.js:406:22:406:25 | name |
445453
| lib/lib.js:405:39:405:42 | name | lib/lib.js:406:22:406:25 | name |
446454
| lib/lib.js:405:39:405:42 | name | lib/lib.js:406:22:406:25 | name |
455+
| lib/lib.js:413:39:413:42 | name | lib/lib.js:414:24:414:27 | name |
456+
| lib/lib.js:413:39:413:42 | name | lib/lib.js:414:24:414:27 | name |
457+
| lib/lib.js:413:39:413:42 | name | lib/lib.js:414:24:414:27 | name |
458+
| lib/lib.js:413:39:413:42 | name | lib/lib.js:414:24:414:27 | name |
459+
| lib/lib.js:418:20:418:23 | name | lib/lib.js:419:25:419:28 | name |
460+
| lib/lib.js:418:20:418:23 | name | lib/lib.js:419:25:419:28 | name |
461+
| lib/lib.js:418:20:418:23 | name | lib/lib.js:419:25:419:28 | name |
462+
| lib/lib.js:418:20:418:23 | name | lib/lib.js:419:25:419:28 | name |
447463
#select
448464
| lib/lib2.js:4:10:4:25 | "rm -rf " + name | lib/lib2.js:3:28:3:31 | name | lib/lib2.js:4:22:4:25 | name | $@ based on library input is later used in $@. | lib/lib2.js:4:10:4:25 | "rm -rf " + name | String concatenation | lib/lib2.js:4:2:4:26 | cp.exec ... + name) | shell command |
449465
| lib/lib2.js:8:10:8:25 | "rm -rf " + name | lib/lib2.js:7:32:7:35 | name | lib/lib2.js:8:22:8:25 | name | $@ based on library input is later used in $@. | lib/lib2.js:8:10:8:25 | "rm -rf " + name | String concatenation | lib/lib2.js:8:2:8:26 | cp.exec ... + name) | shell command |
@@ -502,3 +518,5 @@ edges
502518
| lib/lib.js:351:10:351:27 | "rm -rf " + unsafe | lib/lib.js:349:29:349:34 | unsafe | lib/lib.js:351:22:351:27 | unsafe | $@ based on library input is later used in $@. | lib/lib.js:351:10:351:27 | "rm -rf " + unsafe | String concatenation | lib/lib.js:351:2:351:28 | cp.exec ... unsafe) | shell command |
503519
| lib/lib.js:366:17:366:56 | "learn ... + model | lib/lib.js:360:20:360:23 | opts | lib/lib.js:366:28:366:42 | this.learn_args | $@ based on library input is later used in $@. | lib/lib.js:366:17:366:56 | "learn ... + model | String concatenation | lib/lib.js:367:3:367:18 | cp.exec(command) | shell command |
504520
| lib/lib.js:406:10:406:25 | "rm -rf " + name | lib/lib.js:405:39:405:42 | name | lib/lib.js:406:22:406:25 | name | $@ based on library input is later used in $@. | lib/lib.js:406:10:406:25 | "rm -rf " + name | String concatenation | lib/lib.js:406:2:406:26 | cp.exec ... + name) | shell command |
521+
| lib/lib.js:414:12:414:27 | "rm -rf " + name | lib/lib.js:413:39:413:42 | name | lib/lib.js:414:24:414:27 | name | $@ based on library input is later used in $@. | lib/lib.js:414:12:414:27 | "rm -rf " + name | String concatenation | lib/lib.js:414:2:414:28 | asyncEx ... + name) | shell command |
522+
| lib/lib.js:419:13:419:28 | "rm -rf " + name | lib/lib.js:418:20:418:23 | name | lib/lib.js:419:25:419:28 | name | $@ based on library input is later used in $@. | lib/lib.js:419:13:419:28 | "rm -rf " + name | String concatenation | lib/lib.js:419:3:419:29 | asyncEx ... + name) | shell command |

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,3 +408,35 @@ module.exports.sanitizer3 = function (name) {
408408
var sanitized = yetAnohterSanitizer(name);
409409
cp.exec("rm -rf " + sanitized); // OK
410410
}
411+
412+
var asyncExec = require("async-execute");
413+
module.exports.asyncStuff = function (name) {
414+
asyncExec("rm -rf " + name); // NOT OK
415+
}
416+
417+
const myFuncs = {
418+
myFunc: function (name) {
419+
asyncExec("rm -rf " + name); // NOT OK
420+
}
421+
};
422+
423+
module.exports.blabity = {};
424+
425+
Object.defineProperties(
426+
module.exports.blabity,
427+
Object.assign(
428+
{},
429+
Object.entries(myFuncs).reduce(
430+
(props, [ key, value ]) => Object.assign(
431+
props,
432+
{
433+
[key]: {
434+
value,
435+
configurable: true,
436+
},
437+
},
438+
),
439+
{}
440+
)
441+
)
442+
);

0 commit comments

Comments
 (0)