Merge pull request #863 from MarcMil/fix-flowdroid-bug

Split locals prior to running FlowDroid

Author: StevenArzt

soot-infoflow-android/src/soot/jimple/infoflow/android/source/AndroidSourceSinkManager.java

@@ -440,6 +440,12 @@ protected Collection<ISourceSinkDefinition> getSinkDefinitions(Stmt sCallSite, I
 			final String subSig = callee.getSubSignature();
 			final SootClass sc = callee.getDeclaringClass();
 
+			if (sc == null) {
+				logger.warn(
+						String.format("%s was not declared; was called at %s", callee.getSubSignature(), sCallSite));
+				return sinkDefs;
+			}
+
 			// Do not consider ICC methods as sinks if only the base object is
 			// tainted
 			boolean isParamTainted = false;

soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/generator/SummaryInfoflow.java

@@ -2,6 +2,8 @@
 
 import java.util.Collection;
 
+import soot.Local;
+import soot.jimple.infoflow.FlowDroidLocalSplitter;
 import soot.jimple.infoflow.Infoflow;
 import soot.jimple.infoflow.InfoflowManager;
 import soot.jimple.infoflow.solver.IInfoflowSolver;
@@ -35,10 +37,28 @@ public InfoflowManager getManager() {
 
 	@Override
 	protected void onTaintPropagationCompleted(IInfoflowSolver forwardSolver, IInfoflowSolver aliasSolver,
-											   IInfoflowSolver backwardSolver, IInfoflowSolver backwardAliasSolver) {
+			IInfoflowSolver backwardSolver, IInfoflowSolver backwardAliasSolver) {
 		cachedManager = this.manager;
 	}
 
+	@Override
+	protected void unsplitAllBodies() {
+		//Since we are interested in abstractions, we must not unsplit.
+		//This is fine for our use case
+	}
+
+	@Override
+	protected FlowDroidLocalSplitter getLocalSplitter() {
+		return new FlowDroidLocalSplitter() {
+
+			@Override
+			protected Local createClonedLocal(Local oldLocal) {
+				//We want "normal" locals since we take deep looks into abstractions
+				return (Local) oldLocal.clone();
+			}
+		};
+	}
+
 	@Override
 	protected void initializeSoot(String appPath, String libPath, Collection<String> classes) {
 		this.libPath = libPath;

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;
@@ -495,10 +497,11 @@ protected void constructCallgraph() {
 			// Allow the ICC manager to change the Soot Scene before we continue
 			if (ipcManager != null)
 				ipcManager.updateJimpleForICC();
+			if (config.getAliasingAlgorithm() == AliasingAlgorithm.PtsBased) {
 
-			// We might need to patch invokedynamic instructions
-			if (config.isPatchInvokeDynamicInstructions())
-				patchDynamicInvokeInstructions();
+			}
+
+			patchCode();
 
 			// Run the preprocessors
 			for (PreAnalysisHandler tr : preProcessors)
@@ -512,11 +515,12 @@ 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
@@ -539,20 +543,20 @@ protected void constructCallgraph() {
 	}
 
 	/**
-	 * Re-writes dynamic invocation instructions into traditional invcations
+	 * Inserts patch-code logic
 	 */
-	private void patchDynamicInvokeInstructions() {
+	private void patchCode() {
 		for (SootClass sc : Scene.v().getClasses()) {
 			for (SootMethod sm : sc.getMethods()) {
 				if (sm.hasActiveBody()) {
 					Body body = sm.getActiveBody();
-					patchDynamicInvokeInstructions(body);
+					patchCode(body);
 				} else if (!(sm.getSource() instanceof MethodSourceInjector) && sm.getSource() != null) {
 					sm.setSource(new MethodSourceInjector(sm.getSource()) {
 
 						@Override
 						protected void onMethodSourceLoaded(SootMethod m, Body b) {
-							patchDynamicInvokeInstructions(b);
+							patchCode(b);
 						}
 
 					});
@@ -561,6 +565,17 @@ protected void onMethodSourceLoaded(SootMethod m, Body b) {
 		}
 	}
 
+	private void patchCode(Body body) {
+		if (config.isPatchInvokeDynamicInstructions()) {
+			patchDynamicInvokeInstructions(body);
+		}
+		getLocalSplitter().transform(body);
+	}
+
+	protected FlowDroidLocalSplitter getLocalSplitter() {
+		return FlowDroidLocalSplitter.v();
+	}
+
 	/**
 	 * Patches the dynamic invocation instructions in the given method body
 	 * 
@@ -911,8 +926,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(Scene.v().getReachableMethods().listener());
+				try {
+					runTaintAnalysis(sourcesSinks, additionalSeeds, iCfg, performanceData);
+				} finally {
+					unsplitAllBodies();
+				}
+			}
 
 			// Gather performance data
 			performanceData.setTotalRuntimeSeconds((int) Math.round((System.nanoTime() - beforeCallgraph) / 1E9));
@@ -939,6 +960,48 @@ protected void runAnalysis(final ISourceSinkManager sourcesSinks, final Set<Stri
 		}
 	}
 
+	protected 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(Iterator<? extends MethodOrMethodContext> it) {
+		FlowDroidLocalSplitter splitter = getLocalSplitter();
+		while (it.hasNext()) {
+			MethodOrMethodContext mc = it.next();
+			SootMethod m = mc.method();
+			if (m.isConcrete() && m.getTag(SplittedTag.NAME) == null) {
+				m.addTag(SplittedTag.v());
+				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,70 @@
+package soot.jimple.infoflow;
+
+import java.util.Map;
+
+import soot.Body;
+import soot.Local;
+import soot.Singletons.Global;
+import soot.jimple.internal.JimpleLocal;
+import soot.toolkits.scalar.LocalSplitter;
+import soot.toolkits.scalar.UnusedLocalEliminator;
+
+/**
+ * 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();
+	}
+
+	@Override
+	protected void internalTransform(Body body, String phaseName, Map<String, String> options) {
+		super.internalTransform(body, phaseName, options);
+		UnusedLocalEliminator.v().transform(body);
+	}
+
+}

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

@@ -0,0 +1,19 @@
+package soot.jimple.infoflow;
+
+import soot.tagkit.Tag;
+
+public class SplittedTag implements Tag {
+
+	public static final String NAME = "Splitted";
+	private static final Tag INSTANCE = new SplittedTag();
+
+	@Override
+	public String getName() {
+		return NAME;
+	}
+
+	public static Tag v() {
+		return INSTANCE;
+	}
+
+}

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/data/pathBuilders/BatchPathBuilder.java

@@ -38,6 +38,7 @@ public void computeTaintPaths(Set<AbstractionAtSink> res) {
 		int batchId = 1;
 		long startTime = System.nanoTime();
 		long totalTime = manager.getConfig().getPathConfiguration().getPathReconstructionTotalTime();
+		int completed = 0;
 
 		while (resIt.hasNext()) {
 			// checking if the execution time exceeds the configured totalTime and logging
@@ -86,7 +87,8 @@ public void computeTaintPaths(Set<AbstractionAtSink> res) {
 				resultExecutor.reset();
 			}
 			logger.info("Single batch has used " + (System.nanoTime() - beforeBatch) / 1E9 + " seconds");
-
+			completed += batch.size();
+			reportCompletion(completed, res.size());
 			// If the analysis failed due to an OOM, it doesn't make sense to proceed with
 			// the next batch and get into yet another OOM
 			ISolverTerminationReason currentReason = innerBuilder.getTerminationReason();
@@ -106,6 +108,10 @@ public void computeTaintPaths(Set<AbstractionAtSink> res) {
 		}
 	}
 
+	protected void reportCompletion(int completed, int totalTasks) {
+
+	}
+
 	@Override
 	public InfoflowResults getResults() {
 		return innerBuilder.getResults();

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;
+	}
 }