Merge pull request #623 from timll/develop

StubDroid: do not apply identity on unhandled methods

Author: StevenArzt

soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/taintWrappers/SummaryTaintWrapper.java

@@ -527,9 +527,7 @@ public Set<Abstraction> getTaintsForMethod(Stmt stmt, Abstraction d1, Abstractio
 					return Collections.singleton(taintedAbs);
 				else {
 					reportMissingSummary(callee, stmt, taintedAbs);
-					if (fallbackWrapper != null) {
-						return fallbackWrapper.getTaintsForMethod(stmt, d1, taintedAbs);
-					}
+					return fallbackWrapper != null ? fallbackWrapper.getTaintsForMethod(stmt, d1, taintedAbs) : null;
 				}
 			}
 		}
@@ -1758,14 +1756,21 @@ public Set<Abstraction> getInverseTaintsForMethod(Stmt stmt, Abstraction d1, Abs
 		if (!stmt.containsInvokeExpr())
 			return Collections.singleton(taintedAbs);
 
+		ByReferenceBoolean classSupported = new ByReferenceBoolean(false);
 		// Get the cached data flows
 		final SootMethod method = stmt.getInvokeExpr().getMethod();
-		ClassSummaries flowsInCallees = getFlowSummariesForMethod(stmt, method, taintedAbs, null);
+		ClassSummaries flowsInCallees = getFlowSummariesForMethod(stmt, method, taintedAbs, classSupported);
 
 		// If we have no data flows, we can abort early
-		if (flowsInCallees.isEmpty())
+		if (flowsInCallees.isEmpty()) {
+			if (classSupported.value)
+				return Collections.singleton(taintedAbs);
+
 			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();

soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/taintWrappers/resolvers/SummaryResolver.java

@@ -16,6 +16,7 @@
 import soot.jimple.infoflow.methodSummary.data.provider.IMethodSummaryProvider;
 import soot.jimple.infoflow.methodSummary.data.summary.ClassMethodSummaries;
 import soot.jimple.infoflow.methodSummary.data.summary.ClassSummaries;
+import soot.jimple.infoflow.util.ByReferenceBoolean;
 
 /**
  * A resolver for finding all applicable summaries for a given method call
@@ -35,32 +36,44 @@ public SummaryResponse load(SummaryQuery query) throws Exception {
 					final SootClass declaredClass = query.declaredClass;
 					final String methodSig = query.subsignature;
 					final ClassSummaries classSummaries = new ClassSummaries();
-					boolean isClassSupported = false;
+					boolean directHit = false;
+					ByReferenceBoolean isClassSupported = new ByReferenceBoolean(false);
 
 					// Get the flows in the target method
 					if (calleeClass != null)
-						isClassSupported = getSummaries(methodSig, classSummaries, calleeClass);
+						directHit = getSummaries(methodSig, classSummaries, calleeClass, isClassSupported);
 
 					// If we haven't found any summaries, we look at the class from the declared
 					// type at the call site
-					if (declaredClass != null && !isClassSupported)
-						isClassSupported = getSummaries(methodSig, classSummaries, declaredClass);
+					if (declaredClass != null && !directHit)
+						directHit = getSummaries(methodSig, classSummaries, declaredClass, isClassSupported);
 
 					// If we still don't have anything, we must try the hierarchy. Since this
 					// best-effort approach can be fairly imprecise, it is our last resort.
-					if (!isClassSupported && calleeClass != null)
-						isClassSupported = getSummariesHierarchy(methodSig, classSummaries, calleeClass);
-					if (declaredClass != null && !isClassSupported)
-						isClassSupported = getSummariesHierarchy(methodSig, classSummaries, declaredClass);
-
-					if (isClassSupported) {
-						if (classSummaries.isEmpty())
-							return SummaryResponse.EMPTY_BUT_SUPPORTED;
-						return new SummaryResponse(classSummaries, isClassSupported);
-					} else
+					if (!directHit && calleeClass != null)
+						directHit = getSummariesHierarchy(methodSig, classSummaries, calleeClass, isClassSupported);
+					if (declaredClass != null && !directHit)
+						directHit = getSummariesHierarchy(methodSig, classSummaries, declaredClass, isClassSupported);
+
+					if (directHit && !classSummaries.isEmpty())
+						return new SummaryResponse(classSummaries, true);
+					else if (directHit || isClassSupported.value)
+						return SummaryResponse.EMPTY_BUT_SUPPORTED;
+					else
 						return SummaryResponse.NOT_SUPPORTED;
 				}
 
+				private void updateClassExclusive(ByReferenceBoolean classSupported, SootClass sc, String subsig) {
+					if (classSupported.value)
+						return;
+
+					if (sc.getMethodUnsafe(subsig) == null)
+						return;
+
+					ClassMethodSummaries cms = flows.getClassFlows(sc.getName());
+					classSupported.value |= cms != null && cms.isExclusiveForClass();
+				}
+
 				/**
 				 * Checks whether we have summaries for the given method signature in the given
 				 * class
@@ -70,47 +83,33 @@ public SummaryResponse load(SummaryQuery query) throws Exception {
 				 * @param clazz     The class for which to look for summaries
 				 * @return True if summaries were found, false otherwise
 				 */
-				private boolean getSummaries(final String methodSig, final ClassSummaries summaries, SootClass clazz) {
+				private boolean getSummaries(final String methodSig, final ClassSummaries summaries, SootClass clazz,
+											 ByReferenceBoolean classSupported) {
 					// Do we have direct support for the target class?
 					if (summaries.merge(flows.getMethodFlows(clazz, methodSig)))
 						return true;
 
 					// Do we support any interface this class might have?
-					if (checkInterfaces(methodSig, summaries, clazz))
+					if (checkInterfaces(methodSig, summaries, clazz, classSupported))
 						return true;
 
+					updateClassExclusive(classSupported, clazz, methodSig);
+
+					SootMethod targetMethod = clazz.getMethodUnsafe(methodSig);
 					// If the target is abstract and we haven't found any flows,
 					// we check for child classes
-					SootMethod targetMethod = clazz.getMethodUnsafe(methodSig);
 					if (!clazz.isConcrete() || targetMethod == null || !targetMethod.isConcrete()) {
 						for (SootClass parentClass : getAllParentClasses(clazz)) {
 							// Do we have support for the target class?
 							if (summaries.merge(flows.getMethodFlows(parentClass, methodSig)))
 								return true;
 
 							// Do we support any interface this class might have?
-							if (checkInterfaces(methodSig, summaries, parentClass))
+							if (checkInterfaces(methodSig, summaries, parentClass, classSupported))
 								return true;
-						}
-					}
 
-					// In case we don't have a real hierarchy, we must reconstruct one from the
-					// summaries
-					String curClass = clazz.getName();
-					while (curClass != null) {
-						ClassMethodSummaries classSummaries = flows.getClassFlows(curClass);
-						if (classSummaries != null) {
-							// Check for flows in the current class
-							if (summaries.merge(flows.getMethodFlows(curClass, methodSig)))
-								return true;
-
-							// Check for interfaces
-							if (checkInterfacesFromSummary(methodSig, summaries, curClass))
-								return true;
-
-							curClass = classSummaries.getSuperClass();
-						} else
-							break;
+							updateClassExclusive(classSupported, parentClass, methodSig);
+						}
 					}
 
 					return false;
@@ -126,7 +125,7 @@ private boolean getSummaries(final String methodSig, final ClassSummaries summar
 				 * @return True if summaries were found, false otherwise
 				 */
 				private boolean getSummariesHierarchy(final String methodSig, final ClassSummaries summaries,
-						SootClass clazz) {
+						SootClass clazz, ByReferenceBoolean classSupported) {
 					// Don't try to look up the whole Java hierarchy
 					if (clazz.getName().equals("java.lang.Object"))
 						return false;
@@ -145,9 +144,11 @@ private boolean getSummariesHierarchy(final String methodSig, final ClassSummari
 								found++;
 
 							// Do we support any interface this class might have?
-							if (checkInterfaces(methodSig, summaries, childClass))
+							if (checkInterfaces(methodSig, summaries, childClass, classSupported))
 								found++;
 
+							updateClassExclusive(classSupported, childClass, methodSig);
+
 							// If we have too many summaries that could be applicable, we abort here to
 							// avoid false positives
 							if (found > MAX_HIERARCHY_DEPTH)
@@ -167,7 +168,8 @@ private boolean getSummariesHierarchy(final String methodSig, final ClassSummari
 				 * @param clazz     The interface for which to look for summaries
 				 * @return True if summaries were found, false otherwise
 				 */
-				private boolean checkInterfaces(String methodSig, ClassSummaries summaries, SootClass clazz) {
+				private boolean checkInterfaces(String methodSig, ClassSummaries summaries, SootClass clazz,
+												ByReferenceBoolean classSupported) {
 					for (SootClass intf : clazz.getInterfaces()) {
 						// Directly check the interface
 						if (summaries.merge(flows.getMethodFlows(intf, methodSig)))
@@ -177,42 +179,13 @@ private boolean checkInterfaces(String methodSig, ClassSummaries summaries, Soot
 							// Do we have support for the interface?
 							if (summaries.merge(flows.getMethodFlows(parent, methodSig)))
 								return true;
-						}
-					}
 
-					// We might not have hierarchy information in the scene, so let's check the
-					// summary itself
-					return checkInterfacesFromSummary(methodSig, summaries, clazz.getName());
-				}
-
-				/**
-				 * Checks for summaries on the interfaces implemented by the given class. This
-				 * method relies on the hierarchy data from the summary XML files, rather than
-				 * the Soot scene
-				 * 
-				 * @param methodSig The subsignature of the method for which to get summaries
-				 * @param summaries The summary object to which to add the discovered summaries
-				 * @param className The interface for which to look for summaries
-				 * @return True if summaries were found, false otherwise
-				 */
-				protected boolean checkInterfacesFromSummary(String methodSig, ClassSummaries summaries,
-						String className) {
-					List<String> interfaces = new ArrayList<>();
-					interfaces.add(className);
-					while (!interfaces.isEmpty()) {
-						final String intfName = interfaces.remove(0);
-						ClassMethodSummaries classSummaries = flows.getClassFlows(intfName);
-						if (classSummaries != null && classSummaries.hasInterfaces()) {
-							for (String intf : classSummaries.getInterfaces()) {
-								// Do we have a summary on the current interface?
-								if (summaries.merge(flows.getMethodFlows(intf, methodSig)))
-									return true;
-
-								// Recursively check for more interfaces
-								interfaces.add(intf);
-							}
+							updateClassExclusive(classSupported, parent, methodSig);
 						}
 					}
+
+					// We inject the hierarchy from summaries before the data flow analysis, thus the
+					// soot hierarchy already contains the manual information provided in the xmls.
 					return false;
 				}
 
@@ -246,7 +219,6 @@ private Set<SootClass> getAllChildClasses(SootClass sc) {
 		Set<SootClass> doneSet = new HashSet<SootClass>();
 		Set<SootClass> classes = new HashSet<>();
 
-		final Scene scene = Scene.v();
 		while (!workList.isEmpty()) {
 			SootClass curClass = workList.remove(0);
 			if (!doneSet.add(curClass))
@@ -255,36 +227,13 @@ private Set<SootClass> getAllChildClasses(SootClass sc) {
 			if (curClass.isInterface()) {
 				List<SootClass> hierarchyImplementers = hierarchy.getImplementersOf(curClass);
 				workList.addAll(hierarchyImplementers);
-				if (curClass.isPhantom() && hierarchyImplementers.isEmpty()) {
-					// Query the hierarchy data in the summaries
-					flows.getImplementersOfInterface(curClass.getName()).stream().map(c -> scene.getSootClassUnsafe(c))
-							.filter(c -> c != null).forEach(c -> {
-								workList.add(c);
-							});
-				}
 
 				List<SootClass> subinterfaces = hierarchy.getSubinterfacesOf(curClass);
 				workList.addAll(subinterfaces);
-				if (curClass.isPhantom() && subinterfaces.isEmpty()) {
-					// Query the hierarchy data in the summaries
-					flows.getSubInterfacesOf(curClass.getName()).stream().map(c -> scene.getSootClassUnsafe(c))
-							.filter(c -> c != null).forEach(c -> {
-								workList.add(c);
-							});
-				}
 			} else {
 				List<SootClass> hierarchyClasses = hierarchy.getSubclassesOf(curClass);
 				workList.addAll(hierarchyClasses);
 				classes.add(curClass);
-
-				if (curClass.isPhantom() && hierarchyClasses.isEmpty()) {
-					// Query the hierarchy data in the summaries
-					flows.getSubclassesOf(curClass.getName()).stream().map(c -> scene.getSootClassUnsafe(c))
-							.filter(c -> c != null).forEach(c -> {
-								workList.add(c);
-								classes.add(c);
-							});
-				}
 			}
 		}
 
@@ -305,7 +254,6 @@ private Set<SootClass> getAllParentClasses(SootClass sc) {
 		Set<SootClass> doneSet = new HashSet<SootClass>();
 		Set<SootClass> classes = new HashSet<>();
 
-		final Scene scene = Scene.v();
 		while (!workList.isEmpty()) {
 			SootClass curClass = workList.remove(0);
 			if (!doneSet.add(curClass))
@@ -314,27 +262,10 @@ private Set<SootClass> getAllParentClasses(SootClass sc) {
 			if (curClass.isInterface()) {
 				List<SootClass> hierarchyClasses = hierarchy.getSuperinterfacesOf(curClass);
 				workList.addAll(hierarchyClasses);
-
-				if (curClass.isPhantom() && hierarchyClasses.isEmpty()) {
-					// Query the hierarchy data in the summaries
-					flows.getSuperinterfacesOf(curClass.getName()).stream().map(c -> scene.getSootClassUnsafe(c))
-							.filter(c -> c != null).forEach(c -> {
-								workList.add(c);
-							});
-				}
 			} else {
 				List<SootClass> hierarchyClasses = hierarchy.getSuperclassesOf(curClass);
 				workList.addAll(hierarchyClasses);
 				classes.add(curClass);
-
-				if (curClass.isPhantom() && hierarchyClasses.isEmpty()) {
-					// Query the hierarchy data in the summaries
-					flows.getSuperclassesOf(curClass.getName()).stream().map(c -> scene.getSootClassUnsafe(c))
-							.filter(c -> c != null).forEach(c -> {
-								workList.add(c);
-								classes.add(c);
-							});
-				}
 			}
 		}
 

soot-infoflow-summaries/test/soot/jimple/infoflow/test/methodSummary/ApiClassClient.java

@@ -309,4 +309,22 @@ public void iterator() {
 		if (it.hasNext())
 			sink(it.next());
 	}
+
+	private static void overwrite(Data d) {
+		d.stringField = "";
+	}
+
+	public void noPropagationOverUnhandledCallee() {
+		Data d = new Data();
+		d.stringField = stringSource();
+		overwrite(d);
+		sink(d.stringField);
+	}
+
+	public void identityIsStillAppliedOnUnhandledMethodButExclusiveClass() {
+		Data d = new Data();
+		d.stringField = stringSource();
+		d.identity();
+		sink(d.stringField);
+	}
 }

soot-infoflow-summaries/test/soot/jimple/infoflow/test/methodSummary/Data.java

@@ -52,5 +52,8 @@ public void setI(String i) {
 		this.stringField = i;
 	}
 
-	
+	public void identity() {
+		// NO-OP but do something to make sure that this won't get removed by optimizations
+		System.out.println("Hello World");
+	}
 }

soot-infoflow-summaries/test/soot/jimple/infoflow/test/methodSummary/junit/SummaryTaintWrapperTests.java

@@ -232,6 +232,16 @@ public void iterator() {
 		testFlowForMethod("<soot.jimple.infoflow.test.methodSummary.ApiClassClient: void iterator()>");
 	}
 
+	@Test(timeout = 30000)
+	public void noPropagationOverUnhandledCallee() {
+		testNoFlowForMethod("<soot.jimple.infoflow.test.methodSummary.ApiClassClient: void noPropagationOverUnhandledCallee()>");
+	}
+
+	@Test(timeout = 30000)
+	public void identityIsStillAppliedOnUnhandledMethodButExclusiveClass() {
+		testFlowForMethod("<soot.jimple.infoflow.test.methodSummary.ApiClassClient: void identityIsStillAppliedOnUnhandledMethodButExclusiveClass()>");
+	}
+
 	@Test
 	public void testAllSummaries() throws URISyntaxException, IOException {
 		EagerSummaryProvider provider = new EagerSummaryProvider(TaintWrapperFactory.DEFAULT_SUMMARY_DIR);