Skip to content

Commit 34fe1a8

Browse files
committed
use SSA in the GetLaterAccess module
1 parent 956f991 commit 34fe1a8

File tree

2 files changed

+28
-11
lines changed

2 files changed

+28
-11
lines changed

javascript/ql/lib/semmle/javascript/GlobalAccessPaths.qll

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,16 @@ module AccessPath {
270270

271271
/** A module for computing an access to a variable that happens after a property has been written onto it */
272272
private module GetLaterAccess {
273+
/**
274+
* Gets an reference to the SSA variable `variable`.
275+
* Either the definition or a use of the SSA variable
276+
*/
277+
private VarRef getAVariableRef(SsaVariable variable) {
278+
result = variable.getAUse()
279+
or
280+
result = variable.getDefinition().(SsaExplicitDefinition).getDef().getTarget()
281+
}
282+
273283
/**
274284
* Gets an access to a variable that is written to in `write`, where the access is after the write.
275285
*
@@ -286,7 +296,7 @@ module AccessPath {
286296
pragma[noopt]
287297
DataFlow::Node getLaterBaseAccess(DataFlow::PropWrite write) {
288298
exists(
289-
ControlFlowNode writeNode, BindingPattern access, VarRef otherAccess, Variable variable,
299+
ControlFlowNode writeNode, BindingPattern access, VarRef otherAccess, SsaVariable variable,
290300
StmtContainer container
291301
|
292302
access = getBaseVar(write) and
@@ -323,24 +333,23 @@ module AccessPath {
323333
}
324334

325335
/** Gets an access to `var` inside `container` where `usedInWrite` indicates whether the access is the base of a property write. */
326-
private VarRef getAnAccessInContainer(Variable var, StmtContainer container, boolean usedInWrite) {
327-
result.getVariable() = var and
336+
private VarRef getAnAccessInContainer(
337+
SsaVariable var, StmtContainer container, boolean usedInWrite
338+
) {
339+
result = getAVariableRef(var) and
328340
result.getContainer() = container and
329-
var.isLocal() and
330341
if result = getBaseVar(_) then usedInWrite = true else usedInWrite = false
331342
}
332343

333344
/** Gets a variable that is relevant for the computations in the `GetLaterAccess` module. */
334-
private Variable getARelevantVariable() {
345+
private SsaVariable getARelevantVariable() {
335346
// The variable might be used where `getLaterBaseAccess()` is called.
336347
exists(DataFlow::Node node |
337348
exists(fromRhs(node, _)) and
338-
node.asExpr().(VarAccess).getVariable() = result
349+
node.asExpr() = getAVariableRef(result)
339350
) and
340351
// There is a write that writes to the variable.
341-
getBaseVar(_).getVariable() = result and
342-
// It's local.
343-
result.isLocal() and // we skip global variables, because that turns messy quick.
352+
getBaseVar(_) = getAVariableRef(result) and
344353
// There is both a "write" and "read" in the same container of the variable.
345354
exists(StmtContainer container |
346355
exists(getAnAccessInContainer(result, container, true)) and // a "write", an access to the variable that is the base of a property reference.
@@ -350,9 +359,9 @@ module AccessPath {
350359

351360
/** Gets a basic-block that has a read of the variable that is written to by `write`, where the basicblock occurs after `start`. */
352361
private ReachableBasicBlock getASuccessorBBThatReadsVar(DataFlow::PropWrite write) {
353-
exists(VarAccess baseExpr, Variable var, ControlFlowNode writeNode |
362+
exists(VarRef baseExpr, SsaVariable var, ControlFlowNode writeNode |
354363
baseExpr = getBaseVar(write) and
355-
var = baseExpr.getVariable() and
364+
getAVariableRef(var) = baseExpr and
356365
var = getARelevantVariable() and
357366
writeNode = write.getWriteNode() and
358367
writeNode.getBasicBlock().(ReachableBasicBlock).strictlyDominates(result) and

javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/UnsafeCodeConstruction.expected

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,11 @@ nodes
4242
| lib/index.js:112:17:112:21 | taint |
4343
| lib/index.js:112:17:112:21 | taint |
4444
| lib/index.js:113:20:113:24 | taint |
45+
| lib/index.js:115:38:115:42 | taint |
4546
| lib/index.js:121:34:121:38 | taint |
4647
| lib/index.js:129:32:129:36 | taint |
48+
| lib/index.js:135:23:135:49 | this.op ... dOption |
49+
| lib/index.js:135:23:135:49 | this.op ... dOption |
4750
| lib/index.js:136:23:136:49 | this.op ... dOption |
4851
| lib/index.js:136:23:136:49 | this.op ... dOption |
4952
| lib/index.js:137:23:137:49 | this.op ... dOption |
@@ -94,12 +97,16 @@ edges
9497
| lib/index.js:98:30:98:34 | taint | lib/index.js:105:21:105:47 | this.op ... dOption |
9598
| lib/index.js:112:17:112:21 | taint | lib/index.js:113:20:113:24 | taint |
9699
| lib/index.js:112:17:112:21 | taint | lib/index.js:113:20:113:24 | taint |
100+
| lib/index.js:112:17:112:21 | taint | lib/index.js:115:38:115:42 | taint |
101+
| lib/index.js:112:17:112:21 | taint | lib/index.js:115:38:115:42 | taint |
97102
| lib/index.js:112:17:112:21 | taint | lib/index.js:121:34:121:38 | taint |
98103
| lib/index.js:112:17:112:21 | taint | lib/index.js:121:34:121:38 | taint |
99104
| lib/index.js:112:17:112:21 | taint | lib/index.js:129:32:129:36 | taint |
100105
| lib/index.js:112:17:112:21 | taint | lib/index.js:129:32:129:36 | taint |
101106
| lib/index.js:113:20:113:24 | taint | lib/index.js:138:23:138:32 | this.taint |
102107
| lib/index.js:113:20:113:24 | taint | lib/index.js:138:23:138:32 | this.taint |
108+
| lib/index.js:115:38:115:42 | taint | lib/index.js:135:23:135:49 | this.op ... dOption |
109+
| lib/index.js:115:38:115:42 | taint | lib/index.js:135:23:135:49 | this.op ... dOption |
103110
| lib/index.js:121:34:121:38 | taint | lib/index.js:136:23:136:49 | this.op ... dOption |
104111
| lib/index.js:121:34:121:38 | taint | lib/index.js:136:23:136:49 | this.op ... dOption |
105112
| lib/index.js:129:32:129:36 | taint | lib/index.js:137:23:137:49 | this.op ... dOption |
@@ -114,6 +121,7 @@ edges
114121
| lib/index.js:104:21:104:47 | this.op ... dOption | lib/index.js:86:15:86:19 | taint | lib/index.js:104:21:104:47 | this.op ... dOption | This string concatenation which depends on $@ is later $@. | lib/index.js:86:15:86:19 | taint | library input | lib/index.js:104:10:104:67 | " var ... ing();" | interpreted as code |
115122
| lib/index.js:105:21:105:47 | this.op ... dOption | lib/index.js:86:15:86:19 | taint | lib/index.js:105:21:105:47 | this.op ... dOption | This string concatenation which depends on $@ is later $@. | lib/index.js:86:15:86:19 | taint | library input | lib/index.js:105:10:105:67 | " var ... ing();" | interpreted as code |
116123
| lib/index.js:106:21:106:30 | this.taint | lib/index.js:86:15:86:19 | taint | lib/index.js:106:21:106:30 | this.taint | This string concatenation which depends on $@ is later $@. | lib/index.js:86:15:86:19 | taint | library input | lib/index.js:106:10:106:50 | " var ... ing();" | interpreted as code |
124+
| lib/index.js:135:23:135:49 | this.op ... dOption | lib/index.js:112:17:112:21 | taint | lib/index.js:135:23:135:49 | this.op ... dOption | This string concatenation which depends on $@ is later $@. | lib/index.js:112:17:112:21 | taint | library input | lib/index.js:135:12:135:69 | " var ... ing();" | interpreted as code |
117125
| lib/index.js:136:23:136:49 | this.op ... dOption | lib/index.js:112:17:112:21 | taint | lib/index.js:136:23:136:49 | this.op ... dOption | This string concatenation which depends on $@ is later $@. | lib/index.js:112:17:112:21 | taint | library input | lib/index.js:136:12:136:69 | " var ... ing();" | interpreted as code |
118126
| lib/index.js:137:23:137:49 | this.op ... dOption | lib/index.js:112:17:112:21 | taint | lib/index.js:137:23:137:49 | this.op ... dOption | This string concatenation which depends on $@ is later $@. | lib/index.js:112:17:112:21 | taint | library input | lib/index.js:137:12:137:69 | " var ... ing();" | interpreted as code |
119127
| lib/index.js:138:23:138:32 | this.taint | lib/index.js:112:17:112:21 | taint | lib/index.js:138:23:138:32 | this.taint | This string concatenation which depends on $@ is later $@. | lib/index.js:112:17:112:21 | taint | library input | lib/index.js:138:12:138:52 | " var ... ing();" | interpreted as code |

0 commit comments

Comments
 (0)