Skip to content

Java: Add flow summaries for startActivities #10890

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added data flow summaries for tainted Android intents sent to activities via `Activity.startActivities`.
8 changes: 8 additions & 0 deletions java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ module SummaryComponent {
/** Gets a summary component for `Element`. */
SummaryComponent element() { result = content(any(CollectionContent c)) }

/** Gets a summary component for `ArrayElement`. */
SummaryComponent arrayElement() { result = content(any(ArrayContent c)) }

/** Gets a summary component for `MapValue`. */
SummaryComponent mapValue() { result = content(any(MapValueContent c)) }

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

/** Gets a stack representing `ArrayElement` of `object`. */
SummaryComponentStack arrayElementOf(SummaryComponentStack object) {
result = push(SummaryComponent::arrayElement(), object)
}

/** Gets a stack representing `MapValue` of `object`. */
SummaryComponentStack mapValueOf(SummaryComponentStack object) {
result = push(SummaryComponent::mapValue(), object)
Expand Down
123 changes: 120 additions & 3 deletions java/ql/lib/semmle/code/java/frameworks/android/Intent.qll
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import java
private import semmle.code.java.frameworks.android.Android
private import semmle.code.java.dataflow.DataFlow
private import semmle.code.java.dataflow.ExternalFlow
private import semmle.code.java.dataflow.FlowSteps
private import semmle.code.java.dataflow.FlowSummary
private import semmle.code.java.dataflow.internal.BaseSSA as BaseSsa

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

/** Gets the intent argument of this call. */
Argument getIntentArg() {
result.getType() instanceof TypeIntent and
(
result.getType() instanceof TypeIntent or
result.getType().(Array).getElementType() instanceof TypeIntent
) and
result = this.getAnArgument()
}

/** Holds if this targets a component of type `targetType`. */
predicate targetsComponentType(RefType targetType) {
predicate targetsComponentType(AndroidComponent targetType) {
exists(NewIntent newIntent |
DataFlow::localExprFlow(newIntent, this.getIntentArg()) and
reaches(newIntent, this.getIntentArg()) and
newIntent.getClassArg().getType().(ParameterizedType).getATypeArgument() = targetType
)
}
}

/**
* Holds if `src` reaches the intent argument `arg` of `StartComponentMethodAccess`
* through intra-procedural steps.
*/
private predicate reaches(Expr src, Argument arg) {
any(StartComponentMethodAccess ma).getIntentArg() = arg and
src = arg
or
exists(Expr mid, BaseSsa::BaseSsaVariable ssa, BaseSsa::BaseSsaUpdate upd |
reaches(mid, arg) and
mid = ssa.getAUse() and
upd = ssa.getAnUltimateLocalDefinition() and
src = upd.getDefiningExpr().(VariableAssign).getSource()
)
or
exists(CastingExpr e | e.getExpr() = src | reaches(e, arg))
or
exists(ChooseExpr e | e.getAResultExpr() = src | reaches(e, arg))
or
exists(AssignExpr e | e.getSource() = src | reaches(e, arg))
or
exists(ArrayCreationExpr e | e.getInit().getAnInit() = src | reaches(e, arg))
or
exists(StmtExpr e | e.getResultExpr() = src | reaches(e, arg))
or
exists(NotNullExpr e | e.getExpr() = src | reaches(e, arg))
or
exists(WhenExpr e | e.getBranch(_).getAResult() = src | reaches(e, arg))
}

/**
* A value-preserving step from the intent argument of a `startActivity` call to
* a `getIntent` call in the activity the intent targeted in its constructor.
Expand All @@ -271,6 +307,87 @@ private class StartActivityIntentStep extends AdditionalValueStep {
}
}

/**
* Holds if `targetType` is targeted by an existing `StartComponentMethodAccess` call
* and it's identified by `id`.
*/
private predicate isTargetableType(AndroidComponent targetType, string id) {
exists(StartComponentMethodAccess ma | ma.targetsComponentType(targetType)) and
targetType.getQualifiedName() = id
}

private class StartActivitiesSyntheticCallable extends SyntheticCallable {
AndroidComponent targetType;

StartActivitiesSyntheticCallable() {
exists(string id |
this = "android.content.Activity.startActivities()+" + id and
isTargetableType(targetType, id)
)
}

override StartComponentMethodAccess getACall() {
result.getMethod().hasName("startActivities") and
result.targetsComponentType(targetType)
}

override predicate propagatesFlow(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
exists(ActivityIntentSyntheticGlobal glob | glob.getTargetType() = targetType |
input = SummaryComponentStack::arrayElementOf(SummaryComponentStack::argument(0)) and
output = SummaryComponentStack::singleton(SummaryComponent::syntheticGlobal(glob)) and
preservesValue = true
)
}
}

private class GetIntentSyntheticCallable extends SyntheticCallable {
AndroidComponent targetType;

GetIntentSyntheticCallable() {
exists(string id |
this = "android.content.Activity.getIntent()+" + id and
isTargetableType(targetType, id)
)
}

override Call getACall() {
result.getCallee() instanceof AndroidGetIntentMethod and
result.getEnclosingCallable().getDeclaringType() = targetType
}

override predicate propagatesFlow(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
exists(ActivityIntentSyntheticGlobal glob | glob.getTargetType() = targetType |
input = SummaryComponentStack::singleton(SummaryComponent::syntheticGlobal(glob)) and
output = SummaryComponentStack::return() and
preservesValue = true
)
}
}

private class ActivityIntentSyntheticGlobal extends SummaryComponent::SyntheticGlobal {
AndroidComponent targetType;

ActivityIntentSyntheticGlobal() {
exists(string id |
this = "ActivityIntentSyntheticGlobal+" + id and
isTargetableType(targetType, id)
)
}

AndroidComponent getTargetType() { result = targetType }
}

private class RequiredComponentStackForStartActivities extends RequiredSummaryComponentStack {
override predicate required(SummaryComponent head, SummaryComponentStack tail) {
head = SummaryComponent::arrayElement() and
tail = SummaryComponentStack::argument(0)
}
}

/**
* A value-preserving step from the intent argument of a `sendBroadcast` call to
* the intent parameter in the `onReceive` method of the receiver the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ public void test(Context ctx, Activity act) {
Intent[] intents = new Intent[] {intent};
ctx.startActivities(intents);
}
{
Intent intent = new Intent(null, AnotherActivity.class);
intent.putExtra("data", (String) source("ctx-start-acts-2"));
Intent[] intents = new Intent[] {intent};
ctx.startActivities(intents);
}
{
Intent intent = new Intent(null, SomeActivity.class);
intent.putExtra("data", (String) source("act-start"));
Expand All @@ -35,6 +41,12 @@ public void test(Context ctx, Activity act) {
Intent[] intents = new Intent[] {intent};
act.startActivities(intents);
}
{
Intent intent = new Intent(null, Object.class);
intent.putExtra("data", (String) source("start-activities-should-not-reach"));
Intent[] intents = new Intent[] {intent};
act.startActivities(intents);
}
{
Intent intent = new Intent(null, SomeActivity.class);
intent.putExtra("data", (String) source("start-for-result"));
Expand Down Expand Up @@ -79,9 +91,16 @@ public void test(Context ctx, Activity act) {
static class SomeActivity extends Activity {

public void test() {
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
// @formatter:off
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
// @formatter:on
}
}

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

static class SafeActivity extends Activity {
Expand Down