Skip to content

Commit f5f7259

Browse files
luchua-bcsmowton
authored andcommitted
Revamp the query to implement AdditionalTaintStep
1 parent 3c5c849 commit f5f7259

File tree

2 files changed

+42
-28
lines changed

2 files changed

+42
-28
lines changed

java/ql/src/semmle/code/java/dataflow/FlowSources.qll

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -320,46 +320,31 @@ class AndroidIntentInput extends DataFlow::Node {
320320
}
321321
}
322322

323-
/** Method access to external inputs of `android.content.Intent` object. */
323+
/** Method access to external inputs of `android.content.Intent` or `android.os.BaseBundle` object. */
324324
class IntentGetExtraMethodAccess extends MethodAccess {
325325
IntentGetExtraMethodAccess() {
326326
exists(AndroidComponent ac |
327-
this.getEnclosingCallable().getDeclaringType() = ac and ac.isExported()
328-
) and
329-
(
327+
this.getEnclosingCallable().getDeclaringType() = ac and
328+
ac.isExported() and
330329
this.getMethod().getName().regexpMatch("get\\w+Extra") and
331330
this.getMethod().getDeclaringType() instanceof TypeIntent
332-
or
333-
this.getMethod().getName().regexpMatch("get\\w+") and
334-
this.getQualifier().(MethodAccess).getMethod().hasName("getExtras") and
335-
this.getQualifier().(MethodAccess).getMethod().getDeclaringType() instanceof TypeIntent
336331
)
332+
or
333+
this.getMethod().getName().regexpMatch("get\\w+") and
334+
this
335+
.getMethod()
336+
.getDeclaringType()
337+
.getASupertype*()
338+
.hasQualifiedName("android.os", "BaseBundle")
337339
}
338340
}
339341

340342
/** Android intent extra source. */
341343
private class AndroidIntentExtraSource extends RemoteFlowSource {
342344
AndroidIntentExtraSource() {
343-
exists(MethodAccess ma |
344-
ma instanceof IntentGetExtraMethodAccess and
345-
this.asExpr() = ma and
346-
exists(AndroidIntentInput inode |
347-
(
348-
ma.getQualifier() = inode.asExpr() or // extra from intent
349-
ma.getQualifier() = inode.asParameter().getAnAccess()
350-
)
351-
or
352-
exists(
353-
MethodAccess ema // extra from extras bundle of intent
354-
|
355-
ema.getMethod().hasName("getExtras") and
356-
ma.getQualifier() = ema and
357-
(
358-
ema.getQualifier() = inode.asExpr() or
359-
ema.getQualifier() = inode.asParameter().getAnAccess()
360-
)
361-
)
362-
)
345+
exists(AndroidIntentInput inode |
346+
this.asExpr() = inode.asExpr() or
347+
this.asExpr() = inode.asParameter().getAnAccess()
363348
)
364349
}
365350

java/ql/src/semmle/code/java/dataflow/FlowSteps.qll

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
private import java
66
private import semmle.code.java.dataflow.DataFlow
7+
private import semmle.code.java.dataflow.FlowSources
78

89
/**
910
* A module importing the frameworks that implement additional flow steps,
@@ -139,3 +140,31 @@ private class StringBuilderTaintPreservingCallable extends TaintPreservingCallab
139140
sink = -1
140141
}
141142
}
143+
144+
/**
145+
* Holds if `n1` to `n2` is a dataflow step between the extra getter method and its caller Android `Intent` or `Bundle`.
146+
*/
147+
private predicate intentExtraStep(DataFlow::ExprNode n1, DataFlow::ExprNode n2) {
148+
exists(IntentGetExtraMethodAccess ma |
149+
n1.asExpr() = ma.getQualifier() and
150+
n2.asExpr() = ma
151+
)
152+
}
153+
154+
/**
155+
* Holds if `n1` to `n2` is a dataflow step from Android `Intent` to its `getExtras` method.
156+
*/
157+
private predicate bundleExtraStep(DataFlow::ExprNode n1, DataFlow::ExprNode n2) {
158+
exists(MethodAccess ma | ma.getMethod().hasName("getExtras") |
159+
n1.asExpr() = ma.getQualifier() and
160+
n2.asExpr() = ma
161+
)
162+
}
163+
164+
/** A set of additional taint steps to consider when taint tracking Android intent extra related data flows. */
165+
class AndroidExtraSourceAdditionalTaintStep extends AdditionalTaintStep {
166+
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
167+
intentExtraStep(node1, node2) or
168+
bundleExtraStep(node1, node2)
169+
}
170+
}

0 commit comments

Comments
 (0)