From 935018ae9ec2526a382253397cd7fcdf1caf80c7 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Sat, 26 Nov 2016 16:10:14 -0800 Subject: [PATCH] [quandary] cheaper handling of unknown code Summary: Let's introduce some concepts. A "known unknown" function is one for which no Java code exists (e.g., `native`, `abstract`, and `interface methods`). An "unknown unknown" function is one for which Java code may or may not exist, but we don't have the code or we choose not to analyze it (e.g., non-modeled methods from the core Java or Android libraries). Previously, Quandary handled both known unknowns and unknown unknowns by propagating taint from the parameters of the unknown function to its return value. It turns out that it is really expensive to do this for known unknown functions. D4142697 was the diff that starting handling known unknown functions in this way, and bisecting shows that it was the start of the recent performance problems for Quandary. This diff essentially reverts D4142697 by handling known unknowns as skips instead. Pragmatically, doing the propagation trick for Java/Android library functions (e.g., `String` functions!) matters much more, so i'm not too worried about the missed behaviors from this. Ideally, we will go back to the old handling once performance has improved (have lots of ideas there). But I need this to unblock me in the meantime. Reviewed By: jeremydubreil Differential Revision: D4205507 fbshipit-source-id: 79cb9c8 --- infer/src/quandary/TaintAnalysis.ml | 35 +++++++++--------- .../java/quandary/UnknownCode.java | 36 +++++++++---------- .../codetoanalyze/java/quandary/issues.exp | 9 ++--- 3 files changed, 38 insertions(+), 42 deletions(-) diff --git a/infer/src/quandary/TaintAnalysis.ml b/infer/src/quandary/TaintAnalysis.ml index 0cceec7f6..5c8e5401c 100644 --- a/infer/src/quandary/TaintAnalysis.ml +++ b/infer/src/quandary/TaintAnalysis.ml @@ -21,7 +21,12 @@ module Summary = Summary.Make(struct { payload with Specs.quandary = Some summary; } let read_from_payload payload = - payload.Specs.quandary + match payload.Specs.quandary with + | None -> + (* abstract/native/interface methods will have None as a summary. treat them as skip *) + Some [] + | summary_opt -> + summary_opt end) module Make (TaintSpec : TaintSpec.S) = struct @@ -526,23 +531,17 @@ module Make (TaintSpec : TaintSpec.S) = struct AccessPath.BaseMap.empty formals_with_nums in - let has_body pdesc = - let attrs = Procdesc.get_attributes pdesc in - attrs.is_defined && not attrs.is_abstract in - if has_body pdesc - then - begin - Preanal.doit ~handle_dynamic_dispatch:true pdesc dummy_cg tenv; - let formals = make_formal_access_paths pdesc in - let proc_data = ProcData.make pdesc tenv formals in - match Analyzer.compute_post proc_data with - | Some { access_tree; } -> - let summary = make_summary formals access_tree in - Summary.write_summary (Procdesc.get_proc_name pdesc) summary; - | None -> - if not (Procdesc.is_body_empty pdesc) - then failwith "Couldn't compute post" - end in + Preanal.doit ~handle_dynamic_dispatch:true pdesc dummy_cg tenv; + let formals = make_formal_access_paths pdesc in + let proc_data = ProcData.make pdesc tenv formals in + match Analyzer.compute_post proc_data with + | Some { access_tree; } -> + let summary = make_summary formals access_tree in + Summary.write_summary (Procdesc.get_proc_name pdesc) summary; + | None -> + if Procdesc.Node.get_succs (Procdesc.get_start_node pdesc) <> [] + then failwith "Couldn't compute post" in + let callbacks = { Ondemand.analyze_ondemand; diff --git a/infer/tests/codetoanalyze/java/quandary/UnknownCode.java b/infer/tests/codetoanalyze/java/quandary/UnknownCode.java index 806df16ff..80de59d4a 100644 --- a/infer/tests/codetoanalyze/java/quandary/UnknownCode.java +++ b/infer/tests/codetoanalyze/java/quandary/UnknownCode.java @@ -23,24 +23,6 @@ public abstract class UnknownCode { Object interfaceMethod(Object o); } - void propagateViaUnknownNativeCodeBad() { - Object source = InferTaint.inferSecretSource(); - Object launderedSource = nativeMethod(source); - InferTaint.inferSensitiveSink(launderedSource); - } - - static void propagateViaUnknownAbstractCodeBad() { - Object source = InferTaint.inferSecretSource(); - Object launderedSource = nativeMethod(source); - InferTaint.inferSensitiveSink(launderedSource); - } - - static void propagateViaInterfaceCodeBad(Interface i) { - Object source = InferTaint.inferSecretSource(); - Object launderedSource = i.interfaceMethod(source); - InferTaint.inferSensitiveSink(launderedSource); - } - static void propagateViaUnknownConstructorBad() { String source = (String) InferTaint.inferSecretSource(); // we don't analyze the code for the core Java libraries, so this constructor will be unknown @@ -61,4 +43,22 @@ public abstract class UnknownCode { InferTaint.inferSensitiveSink(launderedSource3); } + static void FN_propagateViaInterfaceCodeBad(Interface i) { + Object source = InferTaint.inferSecretSource(); + Object launderedSource = i.interfaceMethod(source); + InferTaint.inferSensitiveSink(launderedSource); + } + + void FN_propagateViaUnknownNativeCodeBad() { + Object source = InferTaint.inferSecretSource(); + Object launderedSource = nativeMethod(source); + InferTaint.inferSensitiveSink(launderedSource); + } + + static void FN_propagateViaUnknownAbstractCodeBad() { + Object source = InferTaint.inferSecretSource(); + Object launderedSource = nativeMethod(source); + InferTaint.inferSensitiveSink(launderedSource); + } + } diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index 4faf3e352..82c29315a 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/infer/tests/codetoanalyze/java/quandary/issues.exp @@ -31,10 +31,10 @@ DynamicDispatch.java, void DynamicDispatch.FP_propagateViaConcreteTypeOk(), 10, DynamicDispatch.java, void DynamicDispatch.callSinkViaInterfaceBad(DynamicDispatch$Interface), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void DynamicDispatch$BadInterfaceImpl2.callSink(Object),call to void InferTaint.inferSensitiveSink(Object)] DynamicDispatch.java, void DynamicDispatch.callSinkViaInterfaceBad(DynamicDispatch$Interface), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void DynamicDispatch$BadInterfaceImpl1.callSink(Object),call to void InferTaint.inferSensitiveSink(Object)] DynamicDispatch.java, void DynamicDispatch.callSinkViaSubtypeBad(DynamicDispatch$Supertype), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void DynamicDispatch$BadSubtype.callSink(Object),call to void InferTaint.inferSensitiveSink(Object)] -DynamicDispatch.java, void DynamicDispatch.propagateViaInterfaceBad(DynamicDispatch$Interface), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object DynamicDispatch$BadInterfaceImpl1.propagate(Object),flow through Object DynamicDispatch$BadInterfaceImpl2.propagate(Object),flow through Object DynamicDispatch$Interface.propagate(Object),call to void InferTaint.inferSensitiveSink(Object)] +DynamicDispatch.java, void DynamicDispatch.propagateViaInterfaceBad(DynamicDispatch$Interface), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object DynamicDispatch$BadInterfaceImpl1.propagate(Object),flow through Object DynamicDispatch$BadInterfaceImpl2.propagate(Object),call to void InferTaint.inferSensitiveSink(Object)] DynamicDispatch.java, void DynamicDispatch.propagateViaSubtypeBad(DynamicDispatch$Supertype), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object DynamicDispatch$BadSubtype.propagate(Object),call to void InferTaint.inferSensitiveSink(Object)] -DynamicDispatch.java, void DynamicDispatch.returnSourceViaInterfaceBad(DynamicDispatch$Interface), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Object DynamicDispatch$BadInterfaceImpl1.returnSource(),flow through Object DynamicDispatch$Interface.returnSource(),call to void InferTaint.inferSensitiveSink(Object)] -DynamicDispatch.java, void DynamicDispatch.returnSourceViaInterfaceBad(DynamicDispatch$Interface), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Object DynamicDispatch$BadInterfaceImpl2.returnSource(),flow through Object DynamicDispatch$Interface.returnSource(),call to void InferTaint.inferSensitiveSink(Object)] +DynamicDispatch.java, void DynamicDispatch.returnSourceViaInterfaceBad(DynamicDispatch$Interface), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Object DynamicDispatch$BadInterfaceImpl1.returnSource(),call to void InferTaint.inferSensitiveSink(Object)] +DynamicDispatch.java, void DynamicDispatch.returnSourceViaInterfaceBad(DynamicDispatch$Interface), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Object DynamicDispatch$BadInterfaceImpl2.returnSource(),call to void InferTaint.inferSensitiveSink(Object)] DynamicDispatch.java, void DynamicDispatch.returnSourceViaSubtypeBad(DynamicDispatch$Supertype), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Object DynamicDispatch$BadSubtype.returnSource(),call to void InferTaint.inferSensitiveSink(Object)] Exceptions.java, void Exceptions.callSinkThenThrowBad(), 1, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void Exceptions.callSinkThenThrow(Object),call to void InferTaint.inferSensitiveSink(Object)] Exceptions.java, void Exceptions.sinkAfterCatchBad(), 7, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void InferTaint.inferSensitiveSink(Object)] @@ -155,7 +155,4 @@ Strings.java, void Strings.viaStringBufferIgnoreReturnBad(), 4, QUANDARY_TAINT_E Strings.java, void Strings.viaStringBuilderBad(), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through StringBuilder StringBuilder.append(Object),flow through StringBuilder StringBuilder.append(String),flow through String StringBuilder.toString(),call to void InferTaint.inferSensitiveSink(Object)] Strings.java, void Strings.viaStringBuilderIgnoreReturnBad(), 5, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through StringBuilder StringBuilder.append(Object),flow through String StringBuilder.toString(),call to void InferTaint.inferSensitiveSink(Object)] Strings.java, void Strings.viaStringBuilderSugarBad(), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through StringBuilder StringBuilder.append(Object),flow through StringBuilder StringBuilder.append(String),flow through String StringBuilder.toString(),call to void InferTaint.inferSensitiveSink(Object)] -UnknownCode.java, void UnknownCode.propagateViaInterfaceCodeBad(UnknownCode$Interface), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object UnknownCode$Interface.interfaceMethod(Object),call to void InferTaint.inferSensitiveSink(Object)] -UnknownCode.java, void UnknownCode.propagateViaUnknownAbstractCodeBad(), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object UnknownCode.nativeMethod(Object),call to void InferTaint.inferSensitiveSink(Object)] UnknownCode.java, void UnknownCode.propagateViaUnknownConstructorBad(), 4, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through String.(String),call to void InferTaint.inferSensitiveSink(Object)] -UnknownCode.java, void UnknownCode.propagateViaUnknownNativeCodeBad(), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object UnknownCode.nativeMethod(Object),call to void InferTaint.inferSensitiveSink(Object)]