Skip to content

Commit 31e1737

Browse files
committed
Do not add the incoming taint to the outgoing set in the CallToReturn Flow Function if a taint wrapper marked the method as exclusive
Previously, that would happen if the summarized function had no callee.
1 parent bea8788 commit 31e1737

File tree

6 files changed

+29
-30
lines changed

6 files changed

+29
-30
lines changed

soot-infoflow-integration/test/soot/jimple/infoflow/integration/test/junit/AndroidRegressionTests.java

+11
Original file line numberDiff line numberDiff line change
@@ -88,4 +88,15 @@ public void testKotlinAppWithCollections() throws IOException {
8888
Assert.assertEquals(4, results.size());
8989
Assert.assertEquals(4, results.getResultSet().size());
9090
}
91+
92+
/**
93+
* Tests that the CallToReturnFunction does not pass on taints that were killed
94+
* by a taint wrapper that marked the method as exclusive.
95+
*/
96+
@Test
97+
public void testMapClear() throws XmlPullParserException, IOException {
98+
SetupApplication app = initApplication("testAPKs/MapClearTest.apk");
99+
InfoflowResults results = app.runInfoflow("../soot-infoflow-android/SourcesAndSinks.txt");
100+
Assert.assertEquals(0, results.size());
101+
}
91102
}
Binary file not shown.

soot-infoflow/src/soot/jimple/infoflow/problems/BackwardsInfoflowProblem.java

+6-10
Original file line numberDiff line numberDiff line change
@@ -884,17 +884,13 @@ private Set<Abstraction> computeTargetsInternal(Abstraction d1, Abstraction sour
884884
// overwrite them. This is needed e.g. if an heap object
885885
// is an argument and leaked twice in the same path.
886886
// See also Android Source Sink Tests
887-
if (isSink && !manager.getConfig().getInspectSinks()) {
888-
if (source != zeroValue)
889-
res.add(source);
890-
}
887+
if (isSink && !manager.getConfig().getInspectSinks() && source != zeroValue && !killSource.value)
888+
res.add(source);
891889

892-
// If method is excluded, add the taint to not
893-
// break anything
894-
if (isExcluded(callee)) {
895-
if (source != zeroValue)
896-
res.add(source);
897-
}
890+
// If method is excluded, add the taint to not break anything
891+
// unless one of the rules doesn't want so
892+
if (isExcluded(callee) && source != zeroValue && !killSource.value)
893+
res.add(source);
898894

899895
// Static values can be propagated over methods if
900896
// the value isn't written inside the method.

soot-infoflow/src/soot/jimple/infoflow/problems/InfoflowProblem.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -812,8 +812,7 @@ private Set<Abstraction> computeTargetsInternal(Abstraction d1, Abstraction sour
812812
if (passOn && invExpr instanceof InstanceInvokeExpr
813813
&& (manager.getConfig().getInspectSources() || !isSource)
814814
&& (manager.getConfig().getInspectSinks() || !isSink) && !isPrimitiveOrString
815-
&& newSource.getAccessPath().isInstanceFieldRef() && (hasValidCallees
816-
|| (taintWrapper != null && taintWrapper.isExclusive(iCallStmt, newSource)))) {
815+
&& newSource.getAccessPath().isInstanceFieldRef() && hasValidCallees) {
817816
// If one of the callers does not read the value, we
818817
// must pass it on in any case
819818
Collection<SootMethod> callees = interproceduralCFG().getCalleesOfCallAt(call);

soot-infoflow/src/soot/jimple/infoflow/problems/rules/backward/BackwardsWrapperRule.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,6 @@ public Collection<Abstraction> propagateCallToReturnFlow(Abstraction d1, Abstrac
179179
abs = abs.deriveNewAbstractionWithTurnUnit(stmt);
180180
}
181181
}
182-
183-
if (!killSource.value && absAp.equals(sourceAp))
184-
killSource.value = source != abs;
185182
}
186183

187184
if (addAbstraction)
@@ -190,6 +187,10 @@ public Collection<Abstraction> propagateCallToReturnFlow(Abstraction d1, Abstrac
190187
res = resWAliases;
191188
}
192189

190+
// We assume that a taint wrapper returns the complete set of taints for exclusive methods. Thus, if the
191+
// incoming taint should be kept alive, the taint wrapper needs to add it to the outgoing set.
192+
killSource.value |= wrapper.isExclusive(stmt, source);
193+
193194
if (res != null)
194195
for (Abstraction abs : res)
195196
if (abs != source)

soot-infoflow/src/soot/jimple/infoflow/problems/rules/forward/WrapperPropagationRule.java

+7-15
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ public Collection<Abstraction> propagateNormalFlow(Abstraction d1, Abstraction s
4747
* @param source The taint source
4848
* @return The taints computed by the wrapper
4949
*/
50-
private Set<Abstraction> computeWrapperTaints(Abstraction d1, final Stmt iStmt, Abstraction source) {
50+
private Set<Abstraction> computeWrapperTaints(Abstraction d1, final Stmt iStmt, Abstraction source,
51+
ByReferenceBoolean killSource) {
5152
// Do not process zero abstractions
5253
if (source == getZeroValue())
5354
return null;
@@ -102,6 +103,10 @@ private Set<Abstraction> computeWrapperTaints(Abstraction d1, final Stmt iStmt,
102103
res = resWithAliases;
103104
}
104105

106+
// We assume that a taint wrapper returns the complete set of taints for exclusive methods. Thus, if the
107+
// incoming taint should be kept alive, the taint wrapper needs to add it to the outgoing set.
108+
killSource.value = manager.getTaintWrapper() != null && manager.getTaintWrapper().isExclusive(iStmt, source);
109+
105110
return res;
106111
}
107112

@@ -145,20 +150,7 @@ protected void checkAndPropagateAlias(Abstraction d1, final Stmt iStmt, Set<Abst
145150
public Collection<Abstraction> propagateCallToReturnFlow(Abstraction d1, Abstraction source, Stmt stmt,
146151
ByReferenceBoolean killSource, ByReferenceBoolean killAll) {
147152
// Compute the taint wrapper taints
148-
Collection<Abstraction> wrapperTaints = computeWrapperTaints(d1, stmt, source);
149-
if (wrapperTaints != null) {
150-
// If the taint wrapper generated an abstraction for
151-
// the incoming access path, we assume it to be handled
152-
// and do not pass on the incoming abstraction on our own
153-
for (Abstraction wrapperAbs : wrapperTaints)
154-
if (wrapperAbs.getAccessPath().equals(source.getAccessPath())) {
155-
if (wrapperAbs != source)
156-
killSource.value = true;
157-
break;
158-
}
159-
}
160-
161-
return wrapperTaints;
153+
return computeWrapperTaints(d1, stmt, source, killSource);
162154
}
163155

164156
@Override

0 commit comments

Comments
 (0)