Skip to content

Commit ef72c03

Browse files
committed
use simpler taint-step for DestructingPattern
1 parent 733e04c commit ef72c03

3 files changed

Lines changed: 93 additions & 6 deletions

File tree

javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -268,10 +268,10 @@ module TaintTracking {
268268
succ = DataFlow::lvalueNode(fos.getLValue())
269269
)
270270
or
271-
// rest-pattern inside a propety pattern. E.g. from `foo:..` to `args` in `const {foo: {...args}} = something()`.
272-
exists(PropertyPattern pattern |
273-
pred.(DataFlow::PropNode).getAstNode() = pattern and
274-
succ = DataFlow::lvalueNode(pattern.getValuePattern().(ObjectPattern).getRest())
271+
// taint-tracking rest patterns in l-values. E.g. `const {...spread} = foo()` or `const [...spread] = foo()`.
272+
exists(DestructuringPattern pattern |
273+
pred = DataFlow::lvalueNode(pattern) and
274+
succ = DataFlow::lvalueNode(pattern.getRest())
275275
)
276276
}
277277

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

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,44 @@ nodes
8282
| command-line-parameter-command-injection.js:47:8:53:12 | args |
8383
| command-line-parameter-command-injection.js:48:3:50:3 | argv: { ... rgs\\n\\t\\t} |
8484
| command-line-parameter-command-injection.js:48:3:50:3 | argv: { ... rgs\\n\\t\\t} |
85+
| command-line-parameter-command-injection.js:48:9:50:3 | {\\n\\t\\t\\t...args\\n\\t\\t} |
8586
| command-line-parameter-command-injection.js:55:10:55:25 | "cmd.sh " + args |
8687
| command-line-parameter-command-injection.js:55:10:55:25 | "cmd.sh " + args |
8788
| command-line-parameter-command-injection.js:55:22:55:25 | args |
89+
| command-line-parameter-command-injection.js:57:6:57:37 | tainted1 |
90+
| command-line-parameter-command-injection.js:57:17:57:37 | require ... ').argv |
91+
| command-line-parameter-command-injection.js:57:17:57:37 | require ... ').argv |
92+
| command-line-parameter-command-injection.js:58:6:58:40 | tainted2 |
93+
| command-line-parameter-command-injection.js:58:17:58:40 | require ... parse() |
94+
| command-line-parameter-command-injection.js:58:17:58:40 | require ... parse() |
95+
| command-line-parameter-command-injection.js:60:8:63:2 | taint1rest |
96+
| command-line-parameter-command-injection.js:60:8:63:2 | taint2rest |
97+
| command-line-parameter-command-injection.js:60:9:60:31 | taint1: ... t1rest} |
98+
| command-line-parameter-command-injection.js:60:17:60:31 | {...taint1rest} |
99+
| command-line-parameter-command-injection.js:60:33:60:55 | taint2: ... t2rest} |
100+
| command-line-parameter-command-injection.js:60:41:60:55 | {...taint2rest} |
101+
| command-line-parameter-command-injection.js:61:11:61:18 | tainted1 |
102+
| command-line-parameter-command-injection.js:62:11:62:18 | tainted2 |
103+
| command-line-parameter-command-injection.js:65:10:65:31 | "cmd.sh ... nt1rest |
104+
| command-line-parameter-command-injection.js:65:10:65:31 | "cmd.sh ... nt1rest |
105+
| command-line-parameter-command-injection.js:65:22:65:31 | taint1rest |
106+
| command-line-parameter-command-injection.js:66:10:66:31 | "cmd.sh ... nt2rest |
107+
| command-line-parameter-command-injection.js:66:10:66:31 | "cmd.sh ... nt2rest |
108+
| command-line-parameter-command-injection.js:66:22:66:31 | taint2rest |
109+
| command-line-parameter-command-injection.js:68:6:68:16 | {...taint3} |
110+
| command-line-parameter-command-injection.js:68:6:68:40 | taint3 |
111+
| command-line-parameter-command-injection.js:68:20:68:40 | require ... ').argv |
112+
| command-line-parameter-command-injection.js:68:20:68:40 | require ... ').argv |
113+
| command-line-parameter-command-injection.js:69:10:69:27 | "cmd.sh " + taint3 |
114+
| command-line-parameter-command-injection.js:69:10:69:27 | "cmd.sh " + taint3 |
115+
| command-line-parameter-command-injection.js:69:22:69:27 | taint3 |
116+
| command-line-parameter-command-injection.js:71:6:71:16 | [...taint4] |
117+
| command-line-parameter-command-injection.js:71:6:71:40 | taint4 |
118+
| command-line-parameter-command-injection.js:71:20:71:40 | require ... ').argv |
119+
| command-line-parameter-command-injection.js:71:20:71:40 | require ... ').argv |
120+
| command-line-parameter-command-injection.js:72:10:72:27 | "cmd.sh " + taint4 |
121+
| command-line-parameter-command-injection.js:72:10:72:27 | "cmd.sh " + taint4 |
122+
| command-line-parameter-command-injection.js:72:22:72:27 | taint4 |
88123
edges
89124
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
90125
| command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:22:8:36 | process.argv[2] |
@@ -156,10 +191,41 @@ edges
156191
| command-line-parameter-command-injection.js:43:22:43:62 | require ... e().foo | command-line-parameter-command-injection.js:43:10:43:62 | "cmd.sh ... e().foo |
157192
| command-line-parameter-command-injection.js:43:22:43:62 | require ... e().foo | command-line-parameter-command-injection.js:43:10:43:62 | "cmd.sh ... e().foo |
158193
| command-line-parameter-command-injection.js:47:8:53:12 | args | command-line-parameter-command-injection.js:55:22:55:25 | args |
159-
| command-line-parameter-command-injection.js:48:3:50:3 | argv: { ... rgs\\n\\t\\t} | command-line-parameter-command-injection.js:47:8:53:12 | args |
160-
| command-line-parameter-command-injection.js:48:3:50:3 | argv: { ... rgs\\n\\t\\t} | command-line-parameter-command-injection.js:47:8:53:12 | args |
194+
| command-line-parameter-command-injection.js:48:3:50:3 | argv: { ... rgs\\n\\t\\t} | command-line-parameter-command-injection.js:48:9:50:3 | {\\n\\t\\t\\t...args\\n\\t\\t} |
195+
| command-line-parameter-command-injection.js:48:3:50:3 | argv: { ... rgs\\n\\t\\t} | command-line-parameter-command-injection.js:48:9:50:3 | {\\n\\t\\t\\t...args\\n\\t\\t} |
196+
| command-line-parameter-command-injection.js:48:9:50:3 | {\\n\\t\\t\\t...args\\n\\t\\t} | command-line-parameter-command-injection.js:47:8:53:12 | args |
161197
| command-line-parameter-command-injection.js:55:22:55:25 | args | command-line-parameter-command-injection.js:55:10:55:25 | "cmd.sh " + args |
162198
| command-line-parameter-command-injection.js:55:22:55:25 | args | command-line-parameter-command-injection.js:55:10:55:25 | "cmd.sh " + args |
199+
| command-line-parameter-command-injection.js:57:6:57:37 | tainted1 | command-line-parameter-command-injection.js:61:11:61:18 | tainted1 |
200+
| command-line-parameter-command-injection.js:57:17:57:37 | require ... ').argv | command-line-parameter-command-injection.js:57:6:57:37 | tainted1 |
201+
| command-line-parameter-command-injection.js:57:17:57:37 | require ... ').argv | command-line-parameter-command-injection.js:57:6:57:37 | tainted1 |
202+
| command-line-parameter-command-injection.js:58:6:58:40 | tainted2 | command-line-parameter-command-injection.js:62:11:62:18 | tainted2 |
203+
| command-line-parameter-command-injection.js:58:17:58:40 | require ... parse() | command-line-parameter-command-injection.js:58:6:58:40 | tainted2 |
204+
| command-line-parameter-command-injection.js:58:17:58:40 | require ... parse() | command-line-parameter-command-injection.js:58:6:58:40 | tainted2 |
205+
| command-line-parameter-command-injection.js:60:8:63:2 | taint1rest | command-line-parameter-command-injection.js:65:22:65:31 | taint1rest |
206+
| command-line-parameter-command-injection.js:60:8:63:2 | taint2rest | command-line-parameter-command-injection.js:66:22:66:31 | taint2rest |
207+
| command-line-parameter-command-injection.js:60:9:60:31 | taint1: ... t1rest} | command-line-parameter-command-injection.js:60:17:60:31 | {...taint1rest} |
208+
| command-line-parameter-command-injection.js:60:17:60:31 | {...taint1rest} | command-line-parameter-command-injection.js:60:8:63:2 | taint1rest |
209+
| command-line-parameter-command-injection.js:60:33:60:55 | taint2: ... t2rest} | command-line-parameter-command-injection.js:60:41:60:55 | {...taint2rest} |
210+
| command-line-parameter-command-injection.js:60:41:60:55 | {...taint2rest} | command-line-parameter-command-injection.js:60:8:63:2 | taint2rest |
211+
| command-line-parameter-command-injection.js:61:11:61:18 | tainted1 | command-line-parameter-command-injection.js:60:9:60:31 | taint1: ... t1rest} |
212+
| command-line-parameter-command-injection.js:62:11:62:18 | tainted2 | command-line-parameter-command-injection.js:60:33:60:55 | taint2: ... t2rest} |
213+
| command-line-parameter-command-injection.js:65:22:65:31 | taint1rest | command-line-parameter-command-injection.js:65:10:65:31 | "cmd.sh ... nt1rest |
214+
| command-line-parameter-command-injection.js:65:22:65:31 | taint1rest | command-line-parameter-command-injection.js:65:10:65:31 | "cmd.sh ... nt1rest |
215+
| command-line-parameter-command-injection.js:66:22:66:31 | taint2rest | command-line-parameter-command-injection.js:66:10:66:31 | "cmd.sh ... nt2rest |
216+
| command-line-parameter-command-injection.js:66:22:66:31 | taint2rest | command-line-parameter-command-injection.js:66:10:66:31 | "cmd.sh ... nt2rest |
217+
| command-line-parameter-command-injection.js:68:6:68:16 | {...taint3} | command-line-parameter-command-injection.js:68:6:68:40 | taint3 |
218+
| command-line-parameter-command-injection.js:68:6:68:40 | taint3 | command-line-parameter-command-injection.js:69:22:69:27 | taint3 |
219+
| command-line-parameter-command-injection.js:68:20:68:40 | require ... ').argv | command-line-parameter-command-injection.js:68:6:68:16 | {...taint3} |
220+
| command-line-parameter-command-injection.js:68:20:68:40 | require ... ').argv | command-line-parameter-command-injection.js:68:6:68:16 | {...taint3} |
221+
| command-line-parameter-command-injection.js:69:22:69:27 | taint3 | command-line-parameter-command-injection.js:69:10:69:27 | "cmd.sh " + taint3 |
222+
| command-line-parameter-command-injection.js:69:22:69:27 | taint3 | command-line-parameter-command-injection.js:69:10:69:27 | "cmd.sh " + taint3 |
223+
| command-line-parameter-command-injection.js:71:6:71:16 | [...taint4] | command-line-parameter-command-injection.js:71:6:71:40 | taint4 |
224+
| command-line-parameter-command-injection.js:71:6:71:40 | taint4 | command-line-parameter-command-injection.js:72:22:72:27 | taint4 |
225+
| command-line-parameter-command-injection.js:71:20:71:40 | require ... ').argv | command-line-parameter-command-injection.js:71:6:71:16 | [...taint4] |
226+
| command-line-parameter-command-injection.js:71:20:71:40 | require ... ').argv | command-line-parameter-command-injection.js:71:6:71:16 | [...taint4] |
227+
| command-line-parameter-command-injection.js:72:22:72:27 | taint4 | command-line-parameter-command-injection.js:72:10:72:27 | "cmd.sh " + taint4 |
228+
| command-line-parameter-command-injection.js:72:22:72:27 | taint4 | command-line-parameter-command-injection.js:72:10:72:27 | "cmd.sh " + taint4 |
163229
#select
164230
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line argument |
165231
| command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line argument |
@@ -178,3 +244,7 @@ edges
178244
| command-line-parameter-command-injection.js:41:10:41:25 | "cmd.sh " + args | command-line-parameter-command-injection.js:36:13:39:7 | require ... \\t\\t.argv | command-line-parameter-command-injection.js:41:10:41:25 | "cmd.sh " + args | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:36:13:39:7 | require ... \\t\\t.argv | command-line argument |
179245
| command-line-parameter-command-injection.js:43:10:43:62 | "cmd.sh ... e().foo | command-line-parameter-command-injection.js:43:22:43:58 | require ... parse() | command-line-parameter-command-injection.js:43:10:43:62 | "cmd.sh ... e().foo | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:43:22:43:58 | require ... parse() | command-line argument |
180246
| command-line-parameter-command-injection.js:55:10:55:25 | "cmd.sh " + args | command-line-parameter-command-injection.js:48:3:50:3 | argv: { ... rgs\\n\\t\\t} | command-line-parameter-command-injection.js:55:10:55:25 | "cmd.sh " + args | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:48:3:50:3 | argv: { ... rgs\\n\\t\\t} | command-line argument |
247+
| command-line-parameter-command-injection.js:65:10:65:31 | "cmd.sh ... nt1rest | command-line-parameter-command-injection.js:57:17:57:37 | require ... ').argv | command-line-parameter-command-injection.js:65:10:65:31 | "cmd.sh ... nt1rest | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:57:17:57:37 | require ... ').argv | command-line argument |
248+
| command-line-parameter-command-injection.js:66:10:66:31 | "cmd.sh ... nt2rest | command-line-parameter-command-injection.js:58:17:58:40 | require ... parse() | command-line-parameter-command-injection.js:66:10:66:31 | "cmd.sh ... nt2rest | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:58:17:58:40 | require ... parse() | command-line argument |
249+
| command-line-parameter-command-injection.js:69:10:69:27 | "cmd.sh " + taint3 | command-line-parameter-command-injection.js:68:20:68:40 | require ... ').argv | command-line-parameter-command-injection.js:69:10:69:27 | "cmd.sh " + taint3 | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:68:20:68:40 | require ... ').argv | command-line argument |
250+
| command-line-parameter-command-injection.js:72:10:72:27 | "cmd.sh " + taint4 | command-line-parameter-command-injection.js:71:20:71:40 | require ... ').argv | command-line-parameter-command-injection.js:72:10:72:27 | "cmd.sh " + taint4 | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:71:20:71:40 | require ... ').argv | command-line argument |

javascript/ql/test/query-tests/Security/CWE-078/command-line-parameter-command-injection.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,22 @@ cp.exec("cmd.sh " + require("optimist").argv.foo); // NOT OK
5353
.command();
5454

5555
cp.exec("cmd.sh " + args); // NOT OK
56+
57+
var tainted1 = require('yargs').argv;
58+
var tainted2 = require('yargs').parse()
59+
60+
const {taint1: {...taint1rest},taint2: {...taint2rest}} = {
61+
taint1: tainted1,
62+
taint2: tainted2
63+
}
64+
65+
cp.exec("cmd.sh " + taint1rest); // NOT OK - has flow from tainted1
66+
cp.exec("cmd.sh " + taint2rest); // NOT OK - has flow from tianted2
67+
68+
var {...taint3} = require('yargs').argv;
69+
cp.exec("cmd.sh " + taint3); // NOT OK
70+
71+
var [...taint4] = require('yargs').argv;
72+
cp.exec("cmd.sh " + taint4); // NOT OK
5673
});
5774

0 commit comments

Comments
 (0)