Skip to content

Commit baf50c8

Browse files
erik-krogherik-krogh
committed
more precise charpreds in taint steps
1 parent 0f70da2 commit baf50c8

File tree

2 files changed

+115
-108
lines changed

2 files changed

+115
-108
lines changed

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,11 @@ 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, DataFlow::CallNode {
12+
ArrayFunctionTaintStep() { arrayFunctionTaintStep(_, _, this) }
1513

1614
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
17-
arrayFunctionTaintStep(pred, succ, call)
15+
arrayFunctionTaintStep(pred, succ, this)
1816
}
1917
}
2018

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

Lines changed: 112 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -230,42 +230,44 @@ module TaintTracking {
230230
*/
231231
private class HeapTaintStep extends AdditionalTaintStep {
232232
HeapTaintStep() {
233-
this = DataFlow::valueNode(_) or
234-
this = DataFlow::parameterNode(_) or
235-
this instanceof DataFlow::PropRead
233+
heapStep(_, this)
236234
}
237235

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

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

471480
/**

0 commit comments

Comments
 (0)