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 index 6fc0924bd..e35ad2ac7 100644 --- a/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/taintWrappers/SummaryTaintWrapper.java +++ b/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/taintWrappers/SummaryTaintWrapper.java @@ -527,9 +527,7 @@ public Set 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 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(); diff --git a/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/taintWrappers/resolvers/SummaryResolver.java b/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/taintWrappers/resolvers/SummaryResolver.java index 806fd2b9a..8eef9458a 100644 --- a/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/taintWrappers/resolvers/SummaryResolver.java +++ b/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,18 +83,21 @@ 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? @@ -89,28 +105,11 @@ private boolean getSummaries(final String methodSig, final ClassSummaries summar 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 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 getAllChildClasses(SootClass sc) { Set doneSet = new HashSet(); Set 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 getAllChildClasses(SootClass sc) { if (curClass.isInterface()) { List 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 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 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 getAllParentClasses(SootClass sc) { Set doneSet = new HashSet(); Set 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 getAllParentClasses(SootClass sc) { if (curClass.isInterface()) { List 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 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); - }); - } } } 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 index 9d59aa273..ded3eef49 100644 --- a/soot-infoflow-summaries/test/soot/jimple/infoflow/test/methodSummary/ApiClassClient.java +++ b/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); + } } diff --git a/soot-infoflow-summaries/test/soot/jimple/infoflow/test/methodSummary/Data.java b/soot-infoflow-summaries/test/soot/jimple/infoflow/test/methodSummary/Data.java index 49f0e9e57..422a0e65a 100644 --- a/soot-infoflow-summaries/test/soot/jimple/infoflow/test/methodSummary/Data.java +++ b/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"); + } } 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 index dcb1055a2..a5a254f8c 100644 --- 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 @@ -232,6 +232,16 @@ public void iterator() { testFlowForMethod(""); } + @Test(timeout = 30000) + public void noPropagationOverUnhandledCallee() { + testNoFlowForMethod(""); + } + + @Test(timeout = 30000) + public void identityIsStillAppliedOnUnhandledMethodButExclusiveClass() { + testFlowForMethod(""); + } + @Test public void testAllSummaries() throws URISyntaxException, IOException { EagerSummaryProvider provider = new EagerSummaryProvider(TaintWrapperFactory.DEFAULT_SUMMARY_DIR);