Merge pull request #618 from timll/develop

StevenArzt · web-flow · commit 6c47ad3bc79f · 2023-05-24T10:14:12.000+02:00
Various fixes
diff --git a/soot-infoflow-integration/test/soot/jimple/infoflow/integration/test/junit/AndroidRegressionTests.java b/soot-infoflow-integration/test/soot/jimple/infoflow/integration/test/junit/AndroidRegressionTests.java
@@ -88,4 +88,15 @@ public void testKotlinAppWithCollections() throws IOException {
         Assert.assertEquals(4, results.size());
         Assert.assertEquals(4, results.getResultSet().size());
     }
+
+    /**
+     * Tests that the CallToReturnFunction does not pass on taints that were killed
+     * by a taint wrapper that marked the method as exclusive.
+     */
+    @Test
+    public void testMapClear() throws XmlPullParserException, IOException {
+        SetupApplication app = initApplication("testAPKs/MapClearTest.apk");
+        InfoflowResults results = app.runInfoflow("../soot-infoflow-android/SourcesAndSinks.txt");
+        Assert.assertEquals(0, results.size());
+    }
 }
diff --git a/soot-infoflow-integration/testAPKs/MapClearTest.apk b/soot-infoflow-integration/testAPKs/MapClearTest.apk
diff --git a/soot-infoflow-summaries/schema/ClassSummary.xsd b/soot-infoflow-summaries/schema/ClassSummary.xsd
@@ -35,6 +35,7 @@
 		</xs:sequence>
 		<xs:attribute name="fileFormatVersion" type="xs:int" />
 		<xs:attribute name="isInterface" type="xs:boolean" />
+		<xs:attribute name="isExclusive" type="xs:boolean" />
 	</xs:complexType>
 </xs:element>
 
diff --git a/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/generator/SummaryGenerator.java b/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/generator/SummaryGenerator.java
@@ -53,7 +53,6 @@
 import soot.jimple.infoflow.nativeCallHandler.INativeCallHandler;
 import soot.jimple.infoflow.results.InfoflowResults;
 import soot.jimple.infoflow.solver.cfg.IInfoflowCFG;
-import soot.jimple.infoflow.taintWrappers.TaintWrapperList;
 import soot.options.Options;
 
 /**
@@ -75,8 +74,8 @@ public class SummaryGenerator {
 
 	protected List<String> substitutedWith = new LinkedList<String>();
 
-	protected SummaryTaintWrapper fallbackWrapper;
-	protected boolean fallbackWrapperInitialized = false;
+	protected SummaryTaintWrapper summaryTaintWrapper;
+	protected boolean summaryTaintWrapperInitialized = false;
 	protected MemorySummaryProvider onFlySummaryProvider = null;
 
 	public SummaryGenerator() {
@@ -86,10 +85,10 @@ public SummaryGenerator() {
 	/**
 	 * Initializes the fallback taint wrapper
 	 */
-	protected void initializeFallbackWrapper() {
-		if (fallbackWrapperInitialized)
+	protected void initializeSummaryTaintWrapper() {
+		if (summaryTaintWrapperInitialized)
 			return;
-		fallbackWrapperInitialized = true;
+		summaryTaintWrapperInitialized = true;
 
 		try {
 			// Do we want to integrate summaries on the fly?
@@ -113,7 +112,7 @@ protected void initializeFallbackWrapper() {
 
 			// Combine our summary providers
 			IMethodSummaryProvider provider = new MergingSummaryProvider(innerProviders);
-			fallbackWrapper = new SummaryTaintWrapper(provider);
+			summaryTaintWrapper = new SummaryTaintWrapper(provider);
 		} catch (Exception e) {
 			LoggerFactory.getLogger(getClass()).error(
 					"An error occurred while loading the fallback taint wrapper, proceeding without fallback", e);
@@ -125,9 +124,9 @@ protected void initializeFallbackWrapper() {
 	 * new summaries. Call this method after changing pre-existing summary files on
 	 * disk.
 	 */
-	public void releaseFallbackTaintWrapper() {
-		this.fallbackWrapper = null;
-		this.fallbackWrapperInitialized = false;
+	public void releaseSummaryTaintWrapper() {
+		this.summaryTaintWrapper = null;
+		this.summaryTaintWrapperInitialized = false;
 	}
 
 	/**
@@ -625,7 +624,7 @@ protected MethodSummaries createMethodSummary(String classpath, final String met
 			final IGapManager gapManager, final ResultsAvailableHandler resultHandler) {
 		// We need to construct a fallback taint wrapper based on the current
 		// configuration
-		initializeFallbackWrapper();
+		initializeSummaryTaintWrapper();
 
 		logger.info(String.format("Computing method summary for %s...", methodSig));
 		long nanosBeforeMethod = System.nanoTime();
@@ -761,8 +760,12 @@ protected ISummaryInfoflow initInfoflow(MethodSummaries summaries, IGapManager g
 		else
 			iFlow.setNativeCallHandler(new SummaryNativeCallHandler(nativeCallHandler));
 
-		final SummaryGenerationTaintWrapper summaryWrapper = createTaintWrapper(summaries, gapManager);
-		iFlow.setTaintWrapper(new TaintWrapperList(fallbackWrapper, summaryWrapper));
+		final SummaryGenerationTaintWrapper summaryGenWrapper = createTaintWrapper(summaries, gapManager);
+		// We only want to compute summaries for methods we do not have summaries. Thus, we register the
+		// summary generation wrapper as a fallback to the SummaryTaintWrapper such that it is only queried
+		// when the SummaryTaintWrapper doesn't know the method.
+		summaryTaintWrapper.setFallbackTaintWrapper(summaryGenWrapper);
+		iFlow.setTaintWrapper(summaryTaintWrapper);
 
 		// Set the Soot configuration
 		if (sootConfig == null)
diff --git a/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/taintWrappers/SummaryTaintWrapper.java b/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/taintWrappers/SummaryTaintWrapper.java
@@ -7,6 +7,7 @@
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Function;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -526,11 +527,8 @@ public Set<Abstraction> getTaintsForMethod(Stmt stmt, Abstraction d1, Abstractio
 					return Collections.singleton(taintedAbs);
 				else {
 					reportMissingSummary(callee, stmt, taintedAbs);
-					if (fallbackWrapper == null)
-						return null;
-					else {
-						Set<Abstraction> fallbackTaints = fallbackWrapper.getTaintsForMethod(stmt, d1, taintedAbs);
-						return fallbackTaints;
+					if (fallbackWrapper != null) {
+						return fallbackWrapper.getTaintsForMethod(stmt, d1, taintedAbs);
 					}
 				}
 			}
@@ -1594,14 +1592,22 @@ public boolean isExclusive(Stmt stmt, Abstraction taintedPath) {
 
 	@Override
 	public boolean supportsCallee(SootMethod method) {
-		// Check whether we directly support that class. We assume that if we
-		// have some summary for that class, we have all summaries for that
-		// class.
+		// Check whether we directly support that class
 		SootClass declClass = method.getDeclaringClass();
-		if (declClass != null && flows.supportsClass(declClass.getName()))
+		if (declClass == null)
+			return false;
+
+		ClassMethodSummaries cms = flows.getClassFlows(declClass.getName());
+		if (cms == null)
+			return false;
+
+		// We assume for exclusive classes that we have all summaries for that class.
+		if (cms.isExclusiveForClass())
 			return true;
 
-		return false;
+		// Otherwise, we check whether we have a summary for the method.
+		MethodSummaries summaries = cms.getMethodSummaries().filterForMethod(method.getSubSignature());
+		return summaries != null && !summaries.isEmpty();
 	}
 
 	@Override
@@ -1757,12 +1763,9 @@ public Set<Abstraction> getInverseTaintsForMethod(Stmt stmt, Abstraction d1, Abs
 		ClassSummaries flowsInCallees = getFlowSummariesForMethod(stmt, method, taintedAbs, null);
 
 		// If we have no data flows, we can abort early
-		if (flowsInCallees.isEmpty()) {
+		if (flowsInCallees.isEmpty())
 			if (fallbackWrapper != null && fallbackWrapper instanceof IReversibleTaintWrapper)
 				return ((IReversibleTaintWrapper) fallbackWrapper).getInverseTaintsForMethod(stmt, d1, taintedAbs);
-			else
-				return null;
-		}
 
 		// Create a level-0 propagator for the initially tainted access path
 		ByReferenceBoolean killIncomingTaint = new ByReferenceBoolean();
diff --git a/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/xml/SummaryReader.java b/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/xml/SummaryReader.java
@@ -90,6 +90,10 @@ public void read(Reader reader, ClassMethodSummaries summaries)
 					// not call setInterface()!
 					if (isInterface != null && !isInterface.isEmpty())
 						summaries.setInterface(isInterface.equals(XMLConstants.VALUE_TRUE));
+
+					String isExclusive = getAttributeByName(xmlreader, XMLConstants.ATTRIBUTE_IS_EXCLUSIVE);
+					if (isExclusive != null && !isExclusive.isEmpty())
+						summaries.setExclusiveForClass(isExclusive.equals(XMLConstants.VALUE_TRUE));
 				} else if (localName.equals(XMLConstants.TREE_METHODS) && xmlreader.isStartElement()) {
 					if (state == State.summary)
 						state = State.methods;
diff --git a/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/xml/XMLConstants.java b/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/xml/XMLConstants.java
@@ -41,6 +41,7 @@ public class XMLConstants {
 	public static final String ATTRIBUTE_MATCH_STRICT = "matchStrict";
 	public static final String ATTRIBUTE_NAME = "name";
 	public static final String ATTRIBUTE_SUPERCLASS = "superClass";
+	public static final String ATTRIBUTE_IS_EXCLUSIVE = "isExclusive";
 
 	public static final String VALUE_TRUE = "true";
 	public static final String VALUE_FALSE = "false";
diff --git a/soot-infoflow-summaries/summariesManual/kotlin.collections.CollectionsKt.xml b/soot-infoflow-summaries/summariesManual/kotlin.collections.CollectionsKt.xml
@@ -0,0 +1,4 @@
+<?xml version="1.0" ?>
+<summary fileFormatVersion="101">
+    <hierarchy superClass="kotlin.collections.CollectionsKt___CollectionsKt" />
+</summary>
diff --git a/soot-infoflow-summaries/summariesManual/kotlin.collections.CollectionsKt___CollectionsKt.xml b/soot-infoflow-summaries/summariesManual/kotlin.collections.CollectionsKt___CollectionsKt.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" ?>
+<summary fileFormatVersion="101" isExclusive="false">
+    <methods>
+        <method id="java.lang.String joinToString$default(java.lang.Iterable,java.lang.CharSequence,java.lang.CharSequence,java.lang.CharSequence,int,java.lang.CharSequence,kotlin.jvm.functions.Function1,int,java.lang.Object)">
+            <flows>
+                <flow isAlias="false" typeChecking="false" cutSubFields="true">
+                    <from sourceSinkType="Parameter" ParameterIndex="0"
+                          AccessPath="[java.util.Collection: java.lang.Object[] innerArray]"
+                          AccessPathTypes="[java.lang.Object[]]" />
+                    <to sourceSinkType="Return" />
+                </flow>
+            </flows>
+        </method>
+    </methods>
+</summary>
diff --git a/soot-infoflow-summaries/test/soot/jimple/infoflow/test/methodSummary/ApiClassClient.java b/soot-infoflow-summaries/test/soot/jimple/infoflow/test/methodSummary/ApiClassClient.java
@@ -4,8 +4,7 @@
 import java.io.ObjectOutputStream;
 import java.io.ObjectOutputStream.PutField;
 import java.math.BigInteger;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.*;
 
 public class ApiClassClient {
 	public Object source() {
@@ -302,4 +301,12 @@ public void mapToString() {
 		char c = str.charAt(2);
 		sink(c);
 	}
+
+	public void iterator() {
+		List<String> lst = new ArrayList<>();
+		lst.add(stringSource());
+		Iterator<String> it = lst.iterator();
+		if (it.hasNext())
+			sink(it.next());
+	}
 }
diff --git a/soot-infoflow-summaries/test/soot/jimple/infoflow/test/methodSummary/junit/SummaryTaintWrapperTests.java b/soot-infoflow-summaries/test/soot/jimple/infoflow/test/methodSummary/junit/SummaryTaintWrapperTests.java
@@ -227,6 +227,11 @@ public void mapToString() {
 		testFlowForMethod("<soot.jimple.infoflow.test.methodSummary.ApiClassClient: void mapToString()>");
 	}
 
+	@Test(timeout = 30000)
+	public void iterator() {
+		testFlowForMethod("<soot.jimple.infoflow.test.methodSummary.ApiClassClient: void iterator()>");
+	}
+
 	@Test
 	public void testAllSummaries() throws URISyntaxException, IOException {
 		EagerSummaryProvider provider = new EagerSummaryProvider(TaintWrapperFactory.DEFAULT_SUMMARY_DIR);
diff --git a/soot-infoflow/src/soot/jimple/infoflow/problems/AliasProblem.java b/soot-infoflow/src/soot/jimple/infoflow/problems/AliasProblem.java
@@ -814,11 +814,11 @@ public Set<Abstraction> computeTargets(Abstraction d1, Abstraction source) {
 									if (defStmt == null || defStmt.getLeftOp() != abs.getAccessPath().getPlainValue())
 										passOnSet.add(abs);
 
-									// Do not pass on this taint, but
-									// trigger the forward analysis
-									for (Unit u : interproceduralCFG().getPredsOf(call))
-										manager.getMainSolver()
-												.processEdge(new PathEdge<Unit, Abstraction>(d1, u, abs));
+									// Trigger the forward analysis only on new aliases
+									if (!source.equals(abs))
+										for (Unit u : interproceduralCFG().getPredsOf(call))
+											manager.getMainSolver()
+													.processEdge(new PathEdge<Unit, Abstraction>(d1, u, abs));
 								}
 								return notifyOutFlowHandlers(call, d1, source, passOnSet,
 										FlowFunctionType.CallToReturnFlowFunction);
diff --git a/soot-infoflow/src/soot/jimple/infoflow/problems/BackwardsInfoflowProblem.java b/soot-infoflow/src/soot/jimple/infoflow/problems/BackwardsInfoflowProblem.java
@@ -884,17 +884,13 @@ private Set<Abstraction> computeTargetsInternal(Abstraction d1, Abstraction sour
 						// overwrite them. This is needed e.g. if an heap object
 						// is an argument and leaked twice in the same path.
 						// See also Android Source Sink Tests
-						if (isSink && !manager.getConfig().getInspectSinks()) {
-							if (source != zeroValue)
-								res.add(source);
-						}
+						if (isSink && !manager.getConfig().getInspectSinks() && source != zeroValue && !killSource.value)
+							res.add(source);
 
-						// If method is excluded, add the taint to not
-						// break anything
-						if (isExcluded(callee)) {
-							if (source != zeroValue)
-								res.add(source);
-						}
+						// If method is excluded, add the taint to not break anything
+						// unless one of the rules doesn't want so
+						if (isExcluded(callee) && source != zeroValue && !killSource.value)
+							res.add(source);
 
 						// Static values can be propagated over methods if
 						// the value isn't written inside the method.
diff --git a/soot-infoflow/src/soot/jimple/infoflow/problems/InfoflowProblem.java b/soot-infoflow/src/soot/jimple/infoflow/problems/InfoflowProblem.java
@@ -491,6 +491,8 @@ public FlowFunction<Abstraction> getReturnFlowFunction(final Unit callSite, fina
 				// This is not cached by Soot, so accesses are more expensive
 				// than one might think
 				final Local thisLocal = callee.isStatic() ? null : callee.getActiveBody().getThisLocal();
+				final InvokeExpr ie = iCallStmt != null && iCallStmt.containsInvokeExpr() ? iCallStmt.getInvokeExpr() : null;
+				final boolean isExecutorExecute = interproceduralCFG().isExecutorExecute(ie, callee);
 
 				return new SolverReturnFlowFunction() {
 
@@ -668,28 +670,38 @@ private Set<Abstraction> computeTargetsInternal(Abstraction source, Abstraction
 							// check if it is not one of the params
 							// (then we have already fixed it)
 							if (!parameterAliases && !thisAliases && source.getAccessPath().getTaintSubFields()
-									&& iCallStmt.getInvokeExpr() instanceof InstanceInvokeExpr
 									&& aliasing.mayAlias(thisLocal, sourceBase)) {
-								// Type check
-								if (manager.getTypeUtils().checkCast(source.getAccessPath(), thisLocal.getType())) {
-									InstanceInvokeExpr iIExpr = (InstanceInvokeExpr) iCallStmt.getInvokeExpr();
-
-									// Get the caller-side base local
-									// and create a new access path for it
-									Value callerBaseLocal = interproceduralCFG().isReflectiveCallSite(iIExpr)
-											? iIExpr.getArg(0)
-											: iIExpr.getBase();
-									AccessPath ap = manager.getAccessPathFactory().copyWithNewValue(
-											newSource.getAccessPath(), callerBaseLocal,
-											isReflectiveCallSite ? null : newSource.getAccessPath().getBaseType(),
-											false);
-									Abstraction abs = newSource.deriveNewAbstraction(ap, (Stmt) exitStmt);
-									if (abs != null) {
-										res.add(abs);
-										if (!abs.equals(calleeD1))
-											for (Abstraction callerD1 : callerD1s)
-												manager.getAliasing().computeAliases(callerD1, iCallStmt, callerBaseLocal,
-														res, interproceduralCFG().getMethodOf(iCallStmt), abs);
+								if (isExecutorExecute) {
+									if (manager.getTypeUtils().checkCast(source.getAccessPath(),
+																		 ie.getArg(0).getType())) {
+										AccessPath ap = manager.getAccessPathFactory().copyWithNewValue(source.getAccessPath(),
+												ie.getArg(0));
+										Abstraction abs = source.deriveNewAbstraction(ap, (Stmt) exitStmt);
+										if (abs != null)
+											res.add(abs);
+									}
+								} else if (iCallStmt.getInvokeExpr() instanceof InstanceInvokeExpr) {
+									// Type check
+									if (manager.getTypeUtils().checkCast(source.getAccessPath(), thisLocal.getType())) {
+										InstanceInvokeExpr iIExpr = (InstanceInvokeExpr) iCallStmt.getInvokeExpr();
+
+										// Get the caller-side base local
+										// and create a new access path for it
+										Value callerBaseLocal = isReflectiveCallSite
+												? iIExpr.getArg(0)
+												: iIExpr.getBase();
+										AccessPath ap = manager.getAccessPathFactory().copyWithNewValue(
+												newSource.getAccessPath(), callerBaseLocal,
+												isReflectiveCallSite ? null : newSource.getAccessPath().getBaseType(),
+												false);
+										Abstraction abs = newSource.deriveNewAbstraction(ap, (Stmt) exitStmt);
+										if (abs != null) {
+											res.add(abs);
+											if (!abs.equals(calleeD1))
+												for (Abstraction callerD1 : callerD1s)
+													manager.getAliasing().computeAliases(callerD1, iCallStmt, callerBaseLocal,
+															res, interproceduralCFG().getMethodOf(iCallStmt), abs);
+										}
 									}
 								}
 							}
@@ -809,11 +821,10 @@ private Set<Abstraction> computeTargetsInternal(Abstraction d1, Abstraction sour
 						// call/return edges
 						// otherwise we will loose taint - see
 						// ArrayTests/arrayCopyTest
-						if (passOn && invExpr instanceof InstanceInvokeExpr
+						if (passOn && hasValidCallees && !isPrimitiveOrString
 								&& (manager.getConfig().getInspectSources() || !isSource)
-								&& (manager.getConfig().getInspectSinks() || !isSink) && !isPrimitiveOrString
-								&& newSource.getAccessPath().isInstanceFieldRef() && (hasValidCallees
-										|| (taintWrapper != null && taintWrapper.isExclusive(iCallStmt, newSource)))) {
+								&& (manager.getConfig().getInspectSinks() || !isSink)
+								&& newSource.getAccessPath().isInstanceFieldRef()) {
 							// If one of the callers does not read the value, we
 							// must pass it on in any case
 							Collection<SootMethod> callees = interproceduralCFG().getCalleesOfCallAt(call);
@@ -844,23 +855,19 @@ private Set<Abstraction> computeTargetsInternal(Abstraction d1, Abstraction sour
 							}
 
 							if (allCalleesRead) {
-								if (aliasing.mayAlias(((InstanceInvokeExpr) invExpr).getBase(),
-										newSource.getAccessPath().getPlainValue())) {
+								if (invExpr instanceof InstanceInvokeExpr
+										&& aliasing.mayAlias(((InstanceInvokeExpr) invExpr).getBase(),
+															 newSource.getAccessPath().getPlainValue())) {
 									passOn = false;
 								}
-								if (passOn)
-									for (int i = 0; i < callArgs.length; i++)
+								if (passOn) {
+									for (int i = 0; i < callArgs.length; i++) {
 										if (aliasing.mayAlias(callArgs[i], newSource.getAccessPath().getPlainValue())) {
 											passOn = false;
 											break;
 										}
-								// static variables are always propagated if
-								// they are not overwritten. So if we have at
-								// least one call/return edge pair,
-								// we can be sure that the value does not get
-								// "lost" if we do not pass it on:
-								if (newSource.getAccessPath().isStaticFieldRef())
-									passOn = false;
+									}
+								}
 							}
 						}
 
diff --git a/soot-infoflow/src/soot/jimple/infoflow/problems/rules/backward/BackwardsWrapperRule.java b/soot-infoflow/src/soot/jimple/infoflow/problems/rules/backward/BackwardsWrapperRule.java
diff --git a/soot-infoflow/src/soot/jimple/infoflow/problems/rules/forward/WrapperPropagationRule.java b/soot-infoflow/src/soot/jimple/infoflow/problems/rules/forward/WrapperPropagationRule.java
diff --git a/soot-infoflow/src/soot/jimple/infoflow/taintWrappers/TaintWrapperList.java b/soot-infoflow/src/soot/jimple/infoflow/taintWrappers/TaintWrapperList.java
diff --git a/soot-infoflow/test/soot/jimple/infoflow/test/StaticTestCode.java b/soot-infoflow/test/soot/jimple/infoflow/test/StaticTestCode.java
diff --git a/soot-infoflow/test/soot/jimple/infoflow/test/junit/StaticTests.java b/soot-infoflow/test/soot/jimple/infoflow/test/junit/StaticTests.java
