Fix a bug in FlowDroid with reused locals

Author: MarcMil

soot-infoflow/src/soot/jimple/infoflow/AbstractInfoflow.java

@@ -55,7 +55,9 @@
 import soot.jimple.InvokeExpr;
 import soot.jimple.Jimple;
 import soot.jimple.Stmt;
+import soot.jimple.infoflow.FlowDroidLocalSplitter.SplittedLocal;
 import soot.jimple.infoflow.InfoflowConfiguration.AccessPathConfiguration;
+import soot.jimple.infoflow.InfoflowConfiguration.AliasingAlgorithm;
 import soot.jimple.infoflow.InfoflowConfiguration.CallgraphAlgorithm;
 import soot.jimple.infoflow.InfoflowConfiguration.CodeEliminationMode;
 import soot.jimple.infoflow.InfoflowConfiguration.DataFlowDirection;
@@ -512,16 +514,22 @@ protected void constructCallgraph() {
 
 			// To cope with broken APK files, we convert all classes that are still
 			// dangling after resolution into phantoms
-			for (SootClass sc : Scene.v().getClasses())
+			for (SootClass sc : Scene.v().getClasses()) {
 				if (sc.resolvingLevel() == SootClass.DANGLING) {
 					sc.setResolvingLevel(SootClass.BODIES);
 					sc.setPhantomClass();
 				}
+			}
 
 			// We explicitly select the packs we want to run for performance
 			// reasons. Do not re-run the callgraph algorithm if the host
 			// application already provides us with a CG.
 			if (config.getCallgraphAlgorithm() != CallgraphAlgorithm.OnDemand && !Scene.v().hasCallGraph()) {
+				if (config.getAliasingAlgorithm() == AliasingAlgorithm.PtsBased) {
+					//we need to split here already for the PTS to work correctly
+					splitAllBodies();
+				}
+
 				PackManager.v().getPack("wjpp").apply();
 				PackManager.v().getPack("cg").apply();
 			}
@@ -911,8 +919,14 @@ protected void runAnalysis(final ISourceSinkManager sourcesSinks, final Set<Stri
 			IInfoflowCFG iCfg = icfgFactory.buildBiDirICFG(config.getCallgraphAlgorithm(),
 					config.getEnableExceptionTracking());
 
-			if (config.isTaintAnalysisEnabled())
-				runTaintAnalysis(sourcesSinks, additionalSeeds, iCfg, performanceData);
+			if (config.isTaintAnalysisEnabled()) {
+				splitAllBodies();
+				try {
+					runTaintAnalysis(sourcesSinks, additionalSeeds, iCfg, performanceData);
+				} finally {
+					unsplitAllBodies();
+				}
+			}
 
 			// Gather performance data
 			performanceData.setTotalRuntimeSeconds((int) Math.round((System.nanoTime() - beforeCallgraph) / 1E9));
@@ -939,6 +953,47 @@ protected void runAnalysis(final ISourceSinkManager sourcesSinks, final Set<Stri
 		}
 	}
 
+	private void unsplitAllBodies() {
+		for (SootClass sc : Scene.v().getClasses()) {
+			for (SootMethod m : sc.getMethods()) {
+				if (m.hasActiveBody()) {
+					//We could use the local packer here, but we know exactly what was being split
+					//so we can be faster here
+					Body body = m.getActiveBody();
+					Iterator<ValueBox> it = body.getUseAndDefBoxesIterator();
+					while (it.hasNext()) {
+						ValueBox box = it.next();
+						Value val = box.getValue();
+						if (val instanceof SplittedLocal) {
+							SplittedLocal l = (SplittedLocal) val;
+							box.setValue(l.getOriginalLocal());
+						}
+					}
+					Iterator<Local> lit = body.getLocals().iterator();
+					while (lit.hasNext()) {
+						if (lit.next() instanceof SplittedLocal) {
+							lit.remove();
+						}
+					}
+				}
+			}
+		}
+	}
+
+	//With newer soot versions, locals are reused more often, which 
+	//can be a problem for FlowDroid. So, we split the locals prior to 
+	//running FlowDroid.
+	protected void splitAllBodies() {
+		FlowDroidLocalSplitter splitter = FlowDroidLocalSplitter.v();
+		for (SootClass sc : new ArrayList<>(Scene.v().getApplicationClasses())) {
+			for (SootMethod m : new ArrayList<>(sc.getMethods())) {
+				if (m.isConcrete()) {
+					splitter.transform(m.retrieveActiveBody());
+				}
+			}
+		}
+	}
+
 	/**
 	 * Since these simulations are not perfect, we should remove them afterwards
 	 */

soot-infoflow/src/soot/jimple/infoflow/FlowDroidLocalSplitter.java

@@ -0,0 +1,59 @@
+package soot.jimple.infoflow;
+
+import soot.Local;
+import soot.Singletons.Global;
+import soot.jimple.internal.JimpleLocal;
+import soot.toolkits.scalar.LocalSplitter;
+
+/**
+ * With more recent soot versions, locals are reused more often.
+ * This can cause problems in FlowDroid (e.g. the overwriteParameter test case).
+ * The simple solution: We split these locals beforehand
+ * @author Marc Miltenberger
+ */
+public class FlowDroidLocalSplitter extends LocalSplitter {
+	public static class SplittedLocal extends JimpleLocal {
+
+		private static final long serialVersionUID = 1L;
+		private JimpleLocal originalLocal;
+
+		public SplittedLocal(JimpleLocal oldLocal) {
+			super(null, oldLocal.getType());
+			//do not intern the name again
+			setName(oldLocal.getName());
+			if (oldLocal.isUserDefinedLocal()) {
+				setUserDefinedLocal();
+			}
+
+			this.originalLocal = oldLocal;
+			while (originalLocal instanceof SplittedLocal) {
+				originalLocal = ((SplittedLocal) originalLocal).originalLocal;
+			}
+		}
+
+		public JimpleLocal getOriginalLocal() {
+			return originalLocal;
+		}
+
+	}
+
+	public FlowDroidLocalSplitter() {
+		super((Global) null);
+	}
+
+	@Override
+	protected String getNewName(String name, int count) {
+		//Reuse the old name
+		return name;
+	}
+
+	@Override
+	protected Local createClonedLocal(Local oldLocal) {
+		return new SplittedLocal((JimpleLocal) oldLocal);
+	}
+
+	public static FlowDroidLocalSplitter v() {
+		return new FlowDroidLocalSplitter();
+	}
+
+}

soot-infoflow/src/soot/jimple/infoflow/data/AccessPath.java

@@ -586,4 +586,14 @@ public static AccessPath getZeroAccessPath() {
 		return zeroAccessPath;
 	}
 
+	/**
+	 * Creates a new access path that is effectively the same, but based on another local as plain value
+	 * @param newValue the new value
+	 * @return the rebased access path
+	 */
+	public AccessPath rebaseTo(Local newValue) {
+		return new AccessPath(newValue, baseType, fragments, taintSubFields, cutOffApproximation, arrayTaintType,
+				canHaveImmutableAliases);
+	}
+
 }

soot-infoflow/src/soot/jimple/infoflow/results/AbstractResultSourceSinkInfo.java

@@ -1,9 +1,12 @@
 package soot.jimple.infoflow.results;
 
+import soot.Local;
 import soot.jimple.Stmt;
+import soot.jimple.infoflow.FlowDroidLocalSplitter.SplittedLocal;
 import soot.jimple.infoflow.InfoflowConfiguration;
 import soot.jimple.infoflow.data.AccessPath;
 import soot.jimple.infoflow.sourcesSinks.definitions.ISourceSinkDefinition;
+import soot.jimple.internal.JimpleLocal;
 
 /**
  * Abstract base class for information on data flow results
@@ -35,7 +38,7 @@ public AbstractResultSourceSinkInfo(ISourceSinkDefinition definition, AccessPath
 		assert accessPath != null;
 
 		this.definition = definition;
-		this.accessPath = accessPath;
+		this.accessPath = replaceAccessPath(accessPath);
 		this.stmt = stmt;
 		this.userData = userData;
 	}
@@ -97,4 +100,15 @@ public boolean equals(Object o) {
 		return true;
 	}
 
+	protected AccessPath replaceAccessPath(AccessPath accessPath) {
+		Local pv = accessPath.getPlainValue();
+		if (pv instanceof SplittedLocal) {
+			SplittedLocal s = (SplittedLocal) pv;
+			JimpleLocal orig = s.getOriginalLocal();
+
+			AccessPath a = accessPath.rebaseTo(orig);
+			return a;
+		}
+		return accessPath;
+	}
 }

soot-infoflow/src/soot/jimple/infoflow/results/ResultSourceInfo.java

@@ -46,6 +46,7 @@ public ResultSourceInfo(ISourceSinkDefinition definition, AccessPath source, Stm
 		this.pathCallSites = pathCallSites == null || pathCallSites.isEmpty() ? null
 				: pathCallSites.toArray(new Stmt[pathCallSites.size()]);
 		this.pathAgnosticResults = pathAgnosticResults;
+		this.replaceSplitLocalsWithOriginals();
 	}
 
 	public ResultSourceInfo(ISourceSinkDefinition definition, AccessPath source, Stmt context, Object userData,
@@ -56,6 +57,7 @@ public ResultSourceInfo(ISourceSinkDefinition definition, AccessPath source, Stm
 		this.pathAPs = pathAPs;
 		this.pathCallSites = pathCallSites;
 		this.pathAgnosticResults = pathAgnosticResults;
+		this.replaceSplitLocalsWithOriginals();
 	}
 
 	public Stmt[] getPath() {
@@ -97,6 +99,14 @@ public int hashCode() {
 		return result;
 	}
 
+	private void replaceSplitLocalsWithOriginals() {
+		if (pathAPs != null) {
+			for (int i = 0; i < pathAPs.length; i++) {
+				pathAPs[i] = replaceAccessPath(pathAPs[i]);
+			}
+		}
+	}
+
 	@Override
 	public boolean equals(Object obj) {
 		if (this == obj)

soot-infoflow/test/soot/jimple/infoflow/test/junit/JUnitTests.java

@@ -18,6 +18,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 
 import org.junit.Assume;
@@ -26,14 +27,27 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import heros.solver.Pair;
+import soot.Body;
+import soot.Local;
+import soot.Scene;
+import soot.SootClass;
+import soot.SootMethod;
+import soot.Value;
+import soot.ValueBox;
 import soot.jimple.infoflow.AbstractInfoflow;
 import soot.jimple.infoflow.BackwardsInfoflow;
+import soot.jimple.infoflow.FlowDroidLocalSplitter.SplittedLocal;
 import soot.jimple.infoflow.IInfoflow;
 import soot.jimple.infoflow.Infoflow;
 import soot.jimple.infoflow.config.ConfigForTest;
+import soot.jimple.infoflow.data.AccessPath;
 import soot.jimple.infoflow.results.InfoflowResults;
+import soot.jimple.infoflow.results.ResultSinkInfo;
+import soot.jimple.infoflow.results.ResultSourceInfo;
 import soot.jimple.infoflow.taintWrappers.EasyTaintWrapper;
 import soot.jimple.infoflow.test.base.AbstractJUnitTests;
+import soot.util.MultiMap;
 
 /**
  * abstract super class of all test cases which handles initialization, keeps
@@ -115,8 +129,12 @@ public void resetSootAndStream() throws IOException {
 	}
 
 	protected void checkInfoflow(IInfoflow infoflow, int resultCount) {
+		checkCodeForNoSplitLocals();
+		//check that there are no splitted locals left
 		if (infoflow.isResultAvailable()) {
 			InfoflowResults map = infoflow.getResults();
+			checkInfoflowResultsForNoSplitLocals(map);
+
 			assertEquals(resultCount, map.size());
 			assertTrue(map.containsSinkMethod(sink) || map.containsSinkMethod(sinkInt)
 					|| map.containsSinkMethod(sinkBoolean) || map.containsSinkMethod(sinkDouble));
@@ -135,6 +153,55 @@ protected void checkInfoflow(IInfoflow infoflow, int resultCount) {
 		}
 	}
 
+	private void checkCodeForNoSplitLocals() {
+		for (SootClass sc : Scene.v().getClasses()) {
+			for (SootMethod m : sc.getMethods()) {
+				if (m.hasActiveBody()) {
+					Body b = m.getActiveBody();
+					for (Local l : b.getLocals()) {
+						if (l instanceof SplittedLocal) {
+							fail(String.format("Found split local in %s: %s", m.getSignature(), l.getName()));
+						}
+					}
+
+					Iterator<ValueBox> it = b.getUseAndDefBoxesIterator();
+					while (it.hasNext()) {
+						ValueBox vb = it.next();
+						Value v = vb.getValue();
+						if (v instanceof SplittedLocal) {
+							fail(String.format("Found split local in %s: %s", m.getSignature(), v));
+						}
+					}
+				}
+			}
+		}
+	}
+
+	private void checkInfoflowResultsForNoSplitLocals(InfoflowResults map) {
+		MultiMap<ResultSinkInfo, ResultSourceInfo> res = map.getResults();
+		if (res != null) {
+			for (Pair<ResultSinkInfo, ResultSourceInfo> pair : res) {
+				ResultSinkInfo sink = pair.getO1();
+				ResultSourceInfo source = pair.getO2();
+				checkAccessPathForSplitLocals(sink.getAccessPath());
+				checkAccessPathForSplitLocals(source.getAccessPath());
+				AccessPath[] app = source.getPathAccessPaths();
+				if (app != null) {
+					for (AccessPath i : app) {
+						checkAccessPathForSplitLocals(i);
+					}
+				}
+			}
+		}
+	}
+
+	private void checkAccessPathForSplitLocals(AccessPath ap) {
+		Local l = ap.getPlainValue();
+		if (l instanceof SplittedLocal) {
+			fail("Split local found in " + ap);
+		}
+	}
+
 	protected void negativeCheckInfoflow(IInfoflow infoflow) {
 		// If the result is available, it must be empty. Otherwise, it is
 		// implicitly ok since we don't expect to find anything anyway.
@@ -166,7 +233,7 @@ protected IInfoflow initInfoflow(boolean useTaintWrapper) {
 			}
 
 		}
-//		result.getConfig().setLogSourcesAndSinks(true);
+		//		result.getConfig().setLogSourcesAndSinks(true);
 
 		return result;
 	}