Skip to content

Commit 9b34003

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request github#3130 from erik-krogh/PreciseSteps
Approved by asgerf
2 parents 1975a83 + be11418 commit 9b34003

File tree

2 files changed

+116
-110
lines changed

2 files changed

+116
-110
lines changed

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,12 @@ module ArrayTaintTracking {
88
/**
99
* A taint propagating data flow edge caused by the builtin array functions.
1010
*/
11-
private class ArrayFunctionTaintStep extends TaintTracking::AdditionalTaintStep {
12-
DataFlow::CallNode call;
13-
14-
ArrayFunctionTaintStep() { this = call }
11+
private class ArrayFunctionTaintStep extends TaintTracking::AdditionalTaintStep,
12+
DataFlow::CallNode {
13+
ArrayFunctionTaintStep() { arrayFunctionTaintStep(_, _, this) }
1514

1615
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
17-
arrayFunctionTaintStep(pred, succ, call)
16+
arrayFunctionTaintStep(pred, succ, this)
1817
}
1918
}
2019

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

Lines changed: 112 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -229,43 +229,43 @@ module TaintTracking {
229229
* promises.
230230
*/
231231
private class HeapTaintStep extends AdditionalTaintStep {
232-
HeapTaintStep() {
233-
this = DataFlow::valueNode(_) or
234-
this = DataFlow::parameterNode(_) or
235-
this instanceof DataFlow::PropRead
236-
}
232+
HeapTaintStep() { heapStep(_, this) }
237233

238234
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
239-
succ = this and
240-
exists(Expr e, Expr f | e = this.asExpr() and f = pred.asExpr() |
241-
// arrays with tainted elements and objects with tainted property names are tainted
242-
e.(ArrayExpr).getAnElement() = f
243-
or
244-
exists(Property prop | e.(ObjectExpr).getAProperty() = prop |
245-
prop.isComputed() and f = prop.getNameExpr()
246-
)
247-
or
248-
// awaiting a tainted expression gives a tainted result
249-
e.(AwaitExpr).getOperand() = f
250-
or
251-
// spreading a tainted object into an object literal gives a tainted object
252-
e.(ObjectExpr).getAProperty().(SpreadProperty).getInit().(SpreadElement).getOperand() = f
253-
or
254-
// spreading a tainted value into an array literal gives a tainted array
255-
e.(ArrayExpr).getAnElement().(SpreadElement).getOperand() = f
235+
heapStep(pred, succ) and succ = this
236+
}
237+
}
238+
239+
/**
240+
* Holds if there is taint propagation through the heap from `pred` to `succ`.
241+
*/
242+
private predicate heapStep(DataFlow::Node pred, DataFlow::Node succ) {
243+
exists(Expr e, Expr f | e = succ.asExpr() and f = pred.asExpr() |
244+
// arrays with tainted elements and objects with tainted property names are tainted
245+
e.(ArrayExpr).getAnElement() = f
246+
or
247+
exists(Property prop | e.(ObjectExpr).getAProperty() = prop |
248+
prop.isComputed() and f = prop.getNameExpr()
256249
)
257250
or
258-
// reading from a tainted object yields a tainted result
259-
this = succ and
260-
succ.(DataFlow::PropRead).getBase() = pred
251+
// awaiting a tainted expression gives a tainted result
252+
e.(AwaitExpr).getOperand() = f
261253
or
262-
// iterating over a tainted iterator taints the loop variable
263-
exists(ForOfStmt fos |
264-
this = DataFlow::valueNode(fos.getIterationDomain()) and
265-
pred = this and
266-
succ = DataFlow::lvalueNode(fos.getLValue())
267-
)
268-
}
254+
// spreading a tainted object into an object literal gives a tainted object
255+
e.(ObjectExpr).getAProperty().(SpreadProperty).getInit().(SpreadElement).getOperand() = f
256+
or
257+
// spreading a tainted value into an array literal gives a tainted array
258+
e.(ArrayExpr).getAnElement().(SpreadElement).getOperand() = f
259+
)
260+
or
261+
// reading from a tainted object yields a tainted result
262+
succ.(DataFlow::PropRead).getBase() = pred
263+
or
264+
// iterating over a tainted iterator taints the loop variable
265+
exists(ForOfStmt fos |
266+
pred = DataFlow::valueNode(fos.getIterationDomain()) and
267+
succ = DataFlow::lvalueNode(fos.getLValue())
268+
)
269269
}
270270

271271
/**
@@ -388,84 +388,91 @@ module TaintTracking {
388388
* functions defined in the standard library.
389389
*/
390390
private class StringManipulationTaintStep extends AdditionalTaintStep, DataFlow::ValueNode {
391+
StringManipulationTaintStep() { stringManipulationStep(_, this) }
392+
391393
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
392394
succ = this and
395+
stringManipulationStep(pred, succ)
396+
}
397+
}
398+
399+
/**
400+
* Holds if taint can propagate from `pred` to `succ` with a step related to string manipulation.
401+
*/
402+
private predicate stringManipulationStep(DataFlow::Node pred, DataFlow::ValueNode succ) {
403+
// string operations that propagate taint
404+
exists(string name | name = succ.getAstNode().(MethodCallExpr).getMethodName() |
405+
pred.asExpr() = succ.getAstNode().(MethodCallExpr).getReceiver() and
393406
(
394-
// string operations that propagate taint
395-
exists(string name | name = astNode.(MethodCallExpr).getMethodName() |
396-
pred.asExpr() = astNode.(MethodCallExpr).getReceiver() and
397-
(
398-
// sorted, interesting, properties of String.prototype
399-
name = "anchor" or
400-
name = "big" or
401-
name = "blink" or
402-
name = "bold" or
403-
name = "concat" or
404-
name = "fixed" or
405-
name = "fontcolor" or
406-
name = "fontsize" or
407-
name = "italics" or
408-
name = "link" or
409-
name = "padEnd" or
410-
name = "padStart" or
411-
name = "repeat" or
412-
name = "replace" or
413-
name = "slice" or
414-
name = "small" or
415-
name = "split" or
416-
name = "strike" or
417-
name = "sub" or
418-
name = "substr" or
419-
name = "substring" or
420-
name = "sup" or
421-
name = "toLocaleLowerCase" or
422-
name = "toLocaleUpperCase" or
423-
name = "toLowerCase" or
424-
name = "toUpperCase" or
425-
name = "trim" or
426-
name = "trimLeft" or
427-
name = "trimRight" or
428-
// sorted, interesting, properties of Object.prototype
429-
name = "toString" or
430-
name = "valueOf" or
431-
// sorted, interesting, properties of Array.prototype
432-
name = "join"
433-
)
434-
or
435-
exists(int i | pred.asExpr() = astNode.(MethodCallExpr).getArgument(i) |
436-
name = "concat"
437-
or
438-
name = "replace" and i = 1
439-
)
440-
)
441-
or
442-
// standard library constructors that propagate taint: `RegExp` and `String`
443-
exists(DataFlow::InvokeNode invk, string gv | gv = "RegExp" or gv = "String" |
444-
this = invk and
445-
invk = DataFlow::globalVarRef(gv).getAnInvocation() and
446-
pred = invk.getArgument(0)
447-
)
448-
or
449-
// String.fromCharCode and String.fromCodePoint
450-
exists(int i, MethodCallExpr mce |
451-
mce = astNode and
452-
pred.asExpr() = mce.getArgument(i) and
453-
(mce.getMethodName() = "fromCharCode" or mce.getMethodName() = "fromCodePoint")
454-
)
407+
// sorted, interesting, properties of String.prototype
408+
name = "anchor" or
409+
name = "big" or
410+
name = "blink" or
411+
name = "bold" or
412+
name = "concat" or
413+
name = "fixed" or
414+
name = "fontcolor" or
415+
name = "fontsize" or
416+
name = "italics" or
417+
name = "link" or
418+
name = "padEnd" or
419+
name = "padStart" or
420+
name = "repeat" or
421+
name = "replace" or
422+
name = "slice" or
423+
name = "small" or
424+
name = "split" or
425+
name = "strike" or
426+
name = "sub" or
427+
name = "substr" or
428+
name = "substring" or
429+
name = "sup" or
430+
name = "toLocaleLowerCase" or
431+
name = "toLocaleUpperCase" or
432+
name = "toLowerCase" or
433+
name = "toUpperCase" or
434+
name = "trim" or
435+
name = "trimLeft" or
436+
name = "trimRight" or
437+
// sorted, interesting, properties of Object.prototype
438+
name = "toString" or
439+
name = "valueOf" or
440+
// sorted, interesting, properties of Array.prototype
441+
name = "join"
442+
)
443+
or
444+
exists(int i | pred.asExpr() = succ.getAstNode().(MethodCallExpr).getArgument(i) |
445+
name = "concat"
455446
or
456-
// `(encode|decode)URI(Component)?` propagate taint
457-
exists(DataFlow::CallNode c, string name |
458-
this = c and
459-
c = DataFlow::globalVarRef(name).getACall() and
460-
pred = c.getArgument(0)
461-
|
462-
name = "encodeURI" or
463-
name = "decodeURI" or
464-
name = "encodeURIComponent" or
465-
name = "decodeURIComponent"
466-
)
447+
name = "replace" and i = 1
467448
)
468-
}
449+
)
450+
or
451+
// standard library constructors that propagate taint: `RegExp` and `String`
452+
exists(DataFlow::InvokeNode invk, string gv | gv = "RegExp" or gv = "String" |
453+
succ = invk and
454+
invk = DataFlow::globalVarRef(gv).getAnInvocation() and
455+
pred = invk.getArgument(0)
456+
)
457+
or
458+
// String.fromCharCode and String.fromCodePoint
459+
exists(int i, MethodCallExpr mce |
460+
mce = succ.getAstNode() and
461+
pred.asExpr() = mce.getArgument(i) and
462+
(mce.getMethodName() = "fromCharCode" or mce.getMethodName() = "fromCodePoint")
463+
)
464+
or
465+
// `(encode|decode)URI(Component)?` propagate taint
466+
exists(DataFlow::CallNode c, string name |
467+
succ = c and
468+
c = DataFlow::globalVarRef(name).getACall() and
469+
pred = c.getArgument(0)
470+
|
471+
name = "encodeURI" or
472+
name = "decodeURI" or
473+
name = "encodeURIComponent" or
474+
name = "decodeURIComponent"
475+
)
469476
}
470477

471478
/**

0 commit comments

Comments
 (0)