Skip to content

Commit 265f8a3

Browse files
committed
Make bitwise taintsteps specific for this query
1 parent 4e9849e commit 265f8a3

File tree

3 files changed

+29
-12
lines changed

3 files changed

+29
-12
lines changed

java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,6 @@ private predicate localAdditionalTaintExprStep(Expr src, Expr sink) {
176176
serializationStep(src, sink)
177177
or
178178
formatStep(src, sink)
179-
or
180-
bitwiseStep(src, sink)
181179
}
182180

183181
/**
@@ -527,9 +525,6 @@ private class FormatterCallable extends TaintPreservingCallable {
527525
}
528526
}
529527

530-
/** Holds if taint may flow from the operand of a bitwise expression to its result. */
531-
private predicate bitwiseStep(Expr src, BitwiseExpr sink) { sink.(BinaryExpr).getAnOperand() = src }
532-
533528
private import StringBuilderVarModule
534529

535530
module StringBuilderVarModule {

java/ql/lib/semmle/code/java/security/IntentUriPermissionManipulation.qll

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,18 @@ abstract class IntentUriPermissionManipulationSanitizer extends DataFlow::Node {
3131
*/
3232
abstract class IntentUriPermissionManipulationGuard extends DataFlow::BarrierGuard { }
3333

34+
/**
35+
* An additional taint step for flows related to Intent URI permission manipulation
36+
* vulnerabilities.
37+
*/
38+
class IntentUriPermissionManipulationAdditionalTaintStep extends Unit {
39+
/**
40+
* Holds if the step from `node1` to `node2` should be considered a taint
41+
* step for flows related to Intent URI permission manipulation vulnerabilities.
42+
*/
43+
abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
44+
}
45+
3446
private class DefaultIntentUriPermissionManipulationSink extends IntentUriPermissionManipulationSink {
3547
DefaultIntentUriPermissionManipulationSink() {
3648
exists(MethodAccess ma | ma.getMethod() instanceof ActivitySetResultMethod |
@@ -55,14 +67,11 @@ private class IntentFlagsOrDataChangedSanitizer extends IntentUriPermissionManip
5567
this.asExpr() = ma.getQualifier()
5668
|
5769
m.hasName("removeFlags") and
58-
TaintTracking::localExprTaint(any(GrantReadUriPermissionFlag f).getAnAccess(),
59-
ma.getArgument(0)) and
60-
TaintTracking::localExprTaint(any(GrantWriteUriPermissionFlag f).getAnAccess(),
61-
ma.getArgument(0))
70+
bitwiseLocalTaintStep*(any(GrantReadUriPermissionFlag f).getAnAccess(), ma.getArgument(0)) and
71+
bitwiseLocalTaintStep*(any(GrantWriteUriPermissionFlag f).getAnAccess(), ma.getArgument(0))
6272
or
6373
m.hasName("setFlags") and
64-
not TaintTracking::localExprTaint(any(GrantUriPermissionFlag f).getAnAccess(),
65-
ma.getArgument(0))
74+
not bitwiseLocalTaintStep*(any(GrantUriPermissionFlag f).getAnAccess(), ma.getArgument(0))
6675
or
6776
m.hasName("setData")
6877
)
@@ -101,7 +110,7 @@ private predicate intentFlagsOrDataChecked(Guard g, Expr intent, boolean branch)
101110
ma.getMethod() = m and
102111
m.getDeclaringType() instanceof TypeIntent and
103112
m.hasName(["getFlags", "getData"]) and
104-
TaintTracking::localExprTaint(ma, checkedValue)
113+
bitwiseLocalTaintStep*(ma, checkedValue)
105114
|
106115
bitwiseCheck(g, branch) and
107116
checkedValue = g.(EqualityTest).getAnOperand().(AndBitwiseExpr)
@@ -123,3 +132,12 @@ private predicate bitwiseCheck(Guard g, boolean branch) {
123132
else g.(EqualityTest).polarity().booleanNot() = branch
124133
)
125134
}
135+
136+
/**
137+
* Holds if taint can flow from `source` to `sink` in one local step,
138+
* including bitwise operations.
139+
*/
140+
private predicate bitwiseLocalTaintStep(Expr source, Expr sink) {
141+
TaintTracking::localTaintStep(DataFlow::exprNode(source), DataFlow::exprNode(sink)) or
142+
source = sink.(BinaryExpr).getAnOperand()
143+
}

java/ql/lib/semmle/code/java/security/IntentUriPermissionManipulationQuery.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,8 @@ class IntentUriPermissionManipulationConf extends TaintTracking::Configuration {
2727
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
2828
guard instanceof IntentUriPermissionManipulationGuard
2929
}
30+
31+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
32+
any(IntentUriPermissionManipulationAdditionalTaintStep c).step(node1, node2)
33+
}
3034
}

0 commit comments

Comments
 (0)