Skip to content

Commit 429bd5f

Browse files
committed
Add flow summaries for startActivities
Uses SyntheticCallables and SyntheticGlobals to pair each startActivities call to getIntent calls in the components targeted by the intent(s).
1 parent 9e5d9f8 commit 429bd5f

File tree

3 files changed

+148
-4
lines changed

3 files changed

+148
-4
lines changed

java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ module SummaryComponent {
2626
/** Gets a summary component for `Element`. */
2727
SummaryComponent element() { result = content(any(CollectionContent c)) }
2828

29+
/** Gets a summary component for `ArrayElement`. */
30+
SummaryComponent arrayElement() { result = content(any(ArrayContent c)) }
31+
2932
/** Gets a summary component for `MapValue`. */
3033
SummaryComponent mapValue() { result = content(any(MapValueContent c)) }
3134

@@ -52,6 +55,11 @@ module SummaryComponentStack {
5255
result = push(SummaryComponent::element(), object)
5356
}
5457

58+
/** Gets a stack representing `ArrayElement` of `object`. */
59+
SummaryComponentStack arrayElementOf(SummaryComponentStack object) {
60+
result = push(SummaryComponent::arrayElement(), object)
61+
}
62+
5563
/** Gets a stack representing `MapValue` of `object`. */
5664
SummaryComponentStack mapValueOf(SummaryComponentStack object) {
5765
result = push(SummaryComponent::mapValue(), object)

java/ql/lib/semmle/code/java/frameworks/android/Intent.qll

Lines changed: 120 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import java
2+
private import semmle.code.java.frameworks.android.Android
23
private import semmle.code.java.dataflow.DataFlow
34
private import semmle.code.java.dataflow.ExternalFlow
45
private import semmle.code.java.dataflow.FlowSteps
6+
private import semmle.code.java.dataflow.FlowSummary
7+
private import semmle.code.java.dataflow.internal.BaseSSA as BaseSSA
58

69
/** The class `android.content.Intent`. */
710
class TypeIntent extends Class {
@@ -242,19 +245,52 @@ private class StartComponentMethodAccess extends MethodAccess {
242245

243246
/** Gets the intent argument of this call. */
244247
Argument getIntentArg() {
245-
result.getType() instanceof TypeIntent and
248+
(
249+
result.getType() instanceof TypeIntent or
250+
result.getType().(Array).getElementType() instanceof TypeIntent
251+
) and
246252
result = this.getAnArgument()
247253
}
248254

249255
/** Holds if this targets a component of type `targetType`. */
250-
predicate targetsComponentType(RefType targetType) {
256+
predicate targetsComponentType(AndroidComponent targetType) {
251257
exists(NewIntent newIntent |
252-
DataFlow::localExprFlow(newIntent, this.getIntentArg()) and
258+
reaches(newIntent, this.getIntentArg()) and
253259
newIntent.getClassArg().getType().(ParameterizedType).getATypeArgument() = targetType
254260
)
255261
}
256262
}
257263

264+
/**
265+
* Holds if `src` reaches the intent argument `arg` of `StartComponentMethodAccess`
266+
* through intra-procedural steps.
267+
*/
268+
private predicate reaches(Expr src, Argument arg) {
269+
any(StartComponentMethodAccess ma).getIntentArg() = arg and
270+
src = arg
271+
or
272+
exists(Expr mid, BaseSSA::BaseSsaVariable ssa, BaseSSA::BaseSsaUpdate upd |
273+
reaches(mid, arg) and
274+
mid = ssa.getAUse() and
275+
upd = ssa.getAnUltimateLocalDefinition() and
276+
src = upd.getDefiningExpr().(VariableAssign).getSource()
277+
)
278+
or
279+
exists(CastingExpr e | e.getExpr() = src | reaches(e, arg))
280+
or
281+
exists(ChooseExpr e | e.getAResultExpr() = src | reaches(e, arg))
282+
or
283+
exists(AssignExpr e | e.getSource() = src | reaches(e, arg))
284+
or
285+
exists(ArrayCreationExpr e | e.getInit().getAnInit() = src | reaches(e, arg))
286+
or
287+
exists(StmtExpr e | e.getResultExpr() = src | reaches(e, arg))
288+
or
289+
exists(NotNullExpr e | e.getExpr() = src | reaches(e, arg))
290+
or
291+
exists(WhenExpr e | e.getBranch(_).getAResult() = src | reaches(e, arg))
292+
}
293+
258294
/**
259295
* A value-preserving step from the intent argument of a `startActivity` call to
260296
* a `getIntent` call in the activity the intent targeted in its constructor.
@@ -271,6 +307,87 @@ private class StartActivityIntentStep extends AdditionalValueStep {
271307
}
272308
}
273309

310+
/**
311+
* Holds if `targetType` is targeted by an existing `StartComponentMethodAccess` call
312+
* and it's identified by `id`.
313+
*/
314+
private predicate isTargetableType(AndroidComponent targetType, string id) {
315+
exists(StartComponentMethodAccess ma | ma.targetsComponentType(targetType)) and
316+
targetType.getQualifiedName() = id
317+
}
318+
319+
private class StartActivitiesSyntheticCallable extends SyntheticCallable {
320+
AndroidComponent targetType;
321+
322+
StartActivitiesSyntheticCallable() {
323+
exists(string id |
324+
this = "android.content.Activity.startActivities()+" + id and
325+
isTargetableType(targetType, id)
326+
)
327+
}
328+
329+
override StartComponentMethodAccess getACall() {
330+
result.getMethod().hasName("startActivities") and
331+
result.targetsComponentType(targetType)
332+
}
333+
334+
override predicate propagatesFlow(
335+
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
336+
) {
337+
exists(ActivityIntentSyntheticGlobal glob | glob.getTargetType() = targetType |
338+
input = SummaryComponentStack::arrayElementOf(SummaryComponentStack::argument(0)) and
339+
output = SummaryComponentStack::singleton(SummaryComponent::syntheticGlobal(glob)) and
340+
preservesValue = true
341+
)
342+
}
343+
}
344+
345+
private class GetIntentSyntheticCallable extends SyntheticCallable {
346+
AndroidComponent targetType;
347+
348+
GetIntentSyntheticCallable() {
349+
exists(string id |
350+
this = "android.content.Activity.getIntent()+" + id and
351+
isTargetableType(targetType, id)
352+
)
353+
}
354+
355+
override Call getACall() {
356+
result.getCallee() instanceof AndroidGetIntentMethod and
357+
result.getEnclosingCallable().getDeclaringType() = targetType
358+
}
359+
360+
override predicate propagatesFlow(
361+
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
362+
) {
363+
exists(ActivityIntentSyntheticGlobal glob | glob.getTargetType() = targetType |
364+
input = SummaryComponentStack::singleton(SummaryComponent::syntheticGlobal(glob)) and
365+
output = SummaryComponentStack::return() and
366+
preservesValue = true
367+
)
368+
}
369+
}
370+
371+
private class ActivityIntentSyntheticGlobal extends SummaryComponent::SyntheticGlobal {
372+
AndroidComponent targetType;
373+
374+
ActivityIntentSyntheticGlobal() {
375+
exists(string id |
376+
this = "ActivityIntentSyntheticGlobal+" + id and
377+
isTargetableType(targetType, id)
378+
)
379+
}
380+
381+
AndroidComponent getTargetType() { result = targetType }
382+
}
383+
384+
private class RequiredComponentStackForStartActivities extends RequiredSummaryComponentStack {
385+
override predicate required(SummaryComponent head, SummaryComponentStack tail) {
386+
head = SummaryComponent::element() and
387+
tail = SummaryComponentStack::argument(0)
388+
}
389+
}
390+
274391
/**
275392
* A value-preserving step from the intent argument of a `sendBroadcast` call to
276393
* the intent parameter in the `onReceive` method of the receiver the

java/ql/test/library-tests/frameworks/android/intent/TestStartActivityToGetIntent.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ public void test(Context ctx, Activity act) {
2424
Intent[] intents = new Intent[] {intent};
2525
ctx.startActivities(intents);
2626
}
27+
{
28+
Intent intent = new Intent(null, AnotherActivity.class);
29+
intent.putExtra("data", (String) source("ctx-start-acts-2"));
30+
Intent[] intents = new Intent[] {intent};
31+
ctx.startActivities(intents);
32+
}
2733
{
2834
Intent intent = new Intent(null, SomeActivity.class);
2935
intent.putExtra("data", (String) source("act-start"));
@@ -35,6 +41,12 @@ public void test(Context ctx, Activity act) {
3541
Intent[] intents = new Intent[] {intent};
3642
act.startActivities(intents);
3743
}
44+
{
45+
Intent intent = new Intent(null, Object.class);
46+
intent.putExtra("data", (String) source("start-activities-should-not-reach"));
47+
Intent[] intents = new Intent[] {intent};
48+
act.startActivities(intents);
49+
}
3850
{
3951
Intent intent = new Intent(null, SomeActivity.class);
4052
intent.putExtra("data", (String) source("start-for-result"));
@@ -79,9 +91,16 @@ public void test(Context ctx, Activity act) {
7991
static class SomeActivity extends Activity {
8092

8193
public void test() {
82-
sink(getIntent().getStringExtra("data")); // $ hasValueFlow=ctx-start hasValueFlow=act-start hasValueFlow=start-for-result hasValueFlow=start-if-needed hasValueFlow=start-matching hasValueFlow=start-from-child hasValueFlow=start-from-frag hasValueFlow=4-arg MISSING: hasValueFlow=ctx-start-acts hasValueFlow=act-start-acts
94+
// @formatter:off
95+
sink(getIntent().getStringExtra("data")); // $ hasValueFlow=ctx-start hasValueFlow=act-start hasValueFlow=start-for-result hasValueFlow=start-if-needed hasValueFlow=start-matching hasValueFlow=start-from-child hasValueFlow=start-from-frag hasValueFlow=4-arg hasValueFlow=ctx-start-acts hasValueFlow=act-start-acts
96+
// @formatter:on
8397
}
98+
}
8499

100+
static class AnotherActivity extends Activity {
101+
public void test() {
102+
sink(getIntent().getStringExtra("data")); // $ hasValueFlow=ctx-start-acts-2
103+
}
85104
}
86105

87106
static class SafeActivity extends Activity {

0 commit comments

Comments
 (0)