From 874e7f000df786a3d3db52a0e676a1da8f3a491b Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Tue, 1 Nov 2016 08:47:39 -0700 Subject: [PATCH] [quandary] functions that transitively return sources are sources, not passthroughs Summary: D4103759, but for sources instead of sinks Reviewed By: cristianoc Differential Revision: D4104259 fbshipit-source-id: 92ffd2c --- infer/src/quandary/CppTrace.ml | 3 ++ infer/src/quandary/JavaTrace.ml | 3 ++ infer/src/quandary/Sink.ml | 2 - infer/src/quandary/Trace.ml | 44 ++++++++++++------- infer/src/quandary/TraceElem.ml | 1 + infer/src/unit/TraceTests.ml | 4 +- .../codetoanalyze/cpp/quandary/issues.exp | 2 +- .../codetoanalyze/java/quandary/issues.exp | 22 +++++----- 8 files changed, 50 insertions(+), 31 deletions(-) diff --git a/infer/src/quandary/CppTrace.ml b/infer/src/quandary/CppTrace.ml index 17da8819a..04f5f87c9 100644 --- a/infer/src/quandary/CppTrace.ml +++ b/infer/src/quandary/CppTrace.ml @@ -73,6 +73,9 @@ module CppSource = struct | pname -> failwithf "Non-C++ procname %a in C++ analysis@." Procname.pp pname + let to_callee t callee_site = + { t with site = callee_site; } + let compare src1 src2 = SourceKind.compare src1.kind src2.kind |> next CallSite.compare src1.site src2.site diff --git a/infer/src/quandary/JavaTrace.ml b/infer/src/quandary/JavaTrace.ml index 4c37805c9..376b754fe 100644 --- a/infer/src/quandary/JavaTrace.ml +++ b/infer/src/quandary/JavaTrace.ml @@ -81,6 +81,9 @@ module JavaSource = struct | pname when BuiltinDecl.is_declared pname -> None | pname -> failwithf "Non-Java procname %a in Java analysis@." Procname.pp pname + let to_callee t callee_site = + { t with site = callee_site; } + let compare src1 src2 = SourceKind.compare src1.kind src2.kind |> next CallSite.compare src1.site src2.site diff --git a/infer/src/quandary/Sink.ml b/infer/src/quandary/Sink.ml index 3be9b8a2f..26ec12592 100644 --- a/infer/src/quandary/Sink.ml +++ b/infer/src/quandary/Sink.ml @@ -23,8 +23,6 @@ let make_sink_param sink index ~report_reachable = module type S = sig include TraceElem.S - val to_callee : t -> CallSite.t -> t - (** return the parameter index and sink kind for the given call site with the given actuals *) val get : CallSite.t -> (Exp.t * Typ.t) list -> t parameter list end diff --git a/infer/src/quandary/Trace.ml b/infer/src/quandary/Trace.ml index a77f34629..bc38bc0ad 100644 --- a/infer/src/quandary/Trace.ml +++ b/infer/src/quandary/Trace.ml @@ -156,24 +156,38 @@ module Make (Spec : Spec) = struct if is_empty callee_trace then caller_trace else + let non_footprint_callee_sources = + Sources.filter (fun source -> not (Source.is_footprint source)) callee_trace.sources in let sources = - Sources.filter (fun source -> not (Source.is_footprint source)) callee_trace.sources - |> Sources.union caller_trace.sources in - let sinks, passthroughs = - if Sinks.for_all (fun sink -> Sinks.mem sink caller_trace.sinks) callee_trace.sinks + if Sources.subset non_footprint_callee_sources caller_trace.sources then - (* this callee didn't add any new sinks; it's just a passthrough *) - let passthroughs = - Passthroughs.add (Passthrough.make callee_site) caller_trace.passthroughs in - caller_trace.sinks, passthroughs + caller_trace.sources else - (* this callee added a new sink *) - let callee_sinks = - IList.map - (fun sink -> Sink.to_callee sink callee_site) - (Sinks.elements callee_trace.sinks) - |> Sinks.of_list in - Sinks.union caller_trace.sinks callee_sinks, caller_trace.passthroughs in + IList.map + (fun sink -> Source.to_callee sink callee_site) + (Sources.elements non_footprint_callee_sources) + |> Sources.of_list + |> Sources.union caller_trace.sources in + + let sinks = + if Sinks.subset callee_trace.sinks caller_trace.sinks + then + caller_trace.sinks + else + IList.map + (fun sink -> Sink.to_callee sink callee_site) + (Sinks.elements callee_trace.sinks) + |> Sinks.of_list + |> Sinks.union caller_trace.sinks in + + let passthroughs = + if sources == caller_trace.sources && sinks == caller_trace.sinks + then + (* this callee didn't add any new sources or any news sinks; it's just a passthrough *) + Passthroughs.add (Passthrough.make callee_site) caller_trace.passthroughs + else + caller_trace.passthroughs in + { sources; sinks; passthroughs; } let initial = diff --git a/infer/src/quandary/TraceElem.ml b/infer/src/quandary/TraceElem.ml index 1e4952cba..b4fd49907 100644 --- a/infer/src/quandary/TraceElem.ml +++ b/infer/src/quandary/TraceElem.ml @@ -17,6 +17,7 @@ module type S = sig val kind : t -> kind val make : kind -> CallSite.t -> t + val to_callee : t -> CallSite.t -> t val compare : t -> t -> int val equal : t -> t -> bool diff --git a/infer/src/unit/TraceTests.ml b/infer/src/unit/TraceTests.ml index 4f5f85965..058a29479 100644 --- a/infer/src/unit/TraceTests.ml +++ b/infer/src/unit/TraceTests.ml @@ -133,9 +133,9 @@ let tests = "Appended trace should contain source and sink" (MockTrace.equal (MockTrace.append source_trace footprint_trace call_site) expected_trace); - let appended_trace = MockTrace.append MockTrace.initial source_trace call_site in + let appended_trace = MockTrace.append source_trace source_trace call_site in assert_bool - "Appending a trace without a sink should add a passthrough" + "Appending a trace that doesn't add a new source/sink should add a passthrough" (MockTrace.Passthroughs.mem (Passthrough.make call_site) (MockTrace.passthroughs appended_trace)) in "append">::append_ in diff --git a/infer/tests/codetoanalyze/cpp/quandary/issues.exp b/infer/tests/codetoanalyze/cpp/quandary/issues.exp index 2a5cd8681..9538ea4fe 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/issues.exp +++ b/infer/tests/codetoanalyze/cpp/quandary/issues.exp @@ -1,5 +1,5 @@ basics.cpp:28: ERROR: QUANDARY_TAINT_ERROR Error: Other(__infer_taint_source at [line 27]) -> Other(__infer_taint_sink at [line 28]) via { } -basics.cpp:33: ERROR: QUANDARY_TAINT_ERROR Error: Other(__infer_taint_source at [line 20]) -> Other(__infer_taint_sink at [line 33]) via { basics::returnSource at [line 32] } +basics.cpp:33: ERROR: QUANDARY_TAINT_ERROR Error: Other(basics::returnSource at [line 32]) -> Other(__infer_taint_sink at [line 33]) via { } basics.cpp:38: ERROR: QUANDARY_TAINT_ERROR Error: Other(__infer_taint_source at [line 37]) -> Other(basics::callSink at [line 38]) via { } basics.cpp:44: ERROR: QUANDARY_TAINT_ERROR Error: Other(__infer_taint_source at [line 42]) -> Other(basics::callSink at [line 44]) via { basics::id at [line 43] } execs.cpp:52: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 47]) -> ShellExec(execl at [line 52]) via { } diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index 2a289c281..ed624349f 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/infer/tests/codetoanalyze/java/quandary/issues.exp @@ -25,15 +25,15 @@ Basics.java:160: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.infe Basics.java:166: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 164]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 166]) via { } Basics.java:209: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 206]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 209]) via { } Basics.java:218: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 214]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 218]) via { } -DynamicDispatch.java:77: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 25]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 77]) via { Object DynamicDispatch$BadInterfaceImpl1.returnSource() at [line 76], Object DynamicDispatch$BadInterfaceImpl2.returnSource() at [line 76] } -DynamicDispatch.java:77: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 42]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 77]) via { Object DynamicDispatch$BadInterfaceImpl1.returnSource() at [line 76], Object DynamicDispatch$BadInterfaceImpl2.returnSource() at [line 76] } +DynamicDispatch.java:77: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object DynamicDispatch$BadInterfaceImpl1.returnSource() at [line 76]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 77]) via { } +DynamicDispatch.java:77: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object DynamicDispatch$BadInterfaceImpl2.returnSource() at [line 76]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 77]) via { } DynamicDispatch.java:82: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 81]) -> Other(void DynamicDispatch$BadInterfaceImpl1.callSink(Object) at [line 82]) via { } DynamicDispatch.java:82: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 81]) -> Other(void DynamicDispatch$BadInterfaceImpl2.callSink(Object) at [line 82]) via { } DynamicDispatch.java:88: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 86]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 88]) via { Object DynamicDispatch$BadInterfaceImpl1.propagate(Object) at [line 87], Object DynamicDispatch$BadInterfaceImpl2.propagate(Object) at [line 87] } -DynamicDispatch.java:135: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 119]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 135]) via { Object DynamicDispatch$BadSubtype.returnSource() at [line 134] } +DynamicDispatch.java:135: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object DynamicDispatch$BadSubtype.returnSource() at [line 134]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 135]) via { } DynamicDispatch.java:140: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 139]) -> Other(void DynamicDispatch$BadSubtype.callSink(Object) at [line 140]) via { } DynamicDispatch.java:146: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 144]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 146]) via { Object DynamicDispatch$BadSubtype.propagate(Object) at [line 145] } -DynamicDispatch.java:154: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 119]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 154]) via { Object DynamicDispatch$BadSubtype.returnSource() at [line 153] } +DynamicDispatch.java:154: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object DynamicDispatch$BadSubtype.returnSource() at [line 153]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 154]) via { } DynamicDispatch.java:157: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 156]) -> Other(void DynamicDispatch$BadSubtype.callSink(Object) at [line 157]) via { } DynamicDispatch.java:160: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 156]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 160]) via { Object DynamicDispatch$BadSubtype.propagate(Object) at [line 159] } Exceptions.java:23: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 19]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 23]) via { } @@ -80,13 +80,13 @@ Intents.java:51: ERROR: QUANDARY_TAINT_ERROR Error: Intent(Intent Intent.parseIn Intents.java:51: ERROR: QUANDARY_TAINT_ERROR Error: Intent(Intent Intent.parseUri(String,int) at [line 31]) -> Intent(void Activity.startActivityFromFragment(Fragment,Intent,int) at [line 51]) via { } Intents.java:52: ERROR: QUANDARY_TAINT_ERROR Error: Intent(Intent Intent.parseIntent(Resources,XmlPullParser,AttributeSet) at [line 34]) -> Intent(ComponentName ContextWrapper.startService(Intent) at [line 52]) via { } Intents.java:52: ERROR: QUANDARY_TAINT_ERROR Error: Intent(Intent Intent.parseUri(String,int) at [line 31]) -> Intent(ComponentName ContextWrapper.startService(Intent) at [line 52]) via { } -Interprocedural.java:39: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 31]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 39]) via { Object Interprocedural.returnSourceDirect() at [line 39] } -Interprocedural.java:44: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 31]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 44]) via { Object Interprocedural.returnSourceDirect() at [line 43] } -Interprocedural.java:48: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 35]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 48]) via { Object Interprocedural.returnSourceIndirect() at [line 48] } -Interprocedural.java:58: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 53]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 58]) via { Interprocedural$Obj Interprocedural.returnSourceViaField() at [line 58] } -Interprocedural.java:67: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 62]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 67]) via { void Interprocedural.returnSourceViaParameter1(Interprocedural$Obj) at [line 66] } +Interprocedural.java:39: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object Interprocedural.returnSourceDirect() at [line 39]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 39]) via { } +Interprocedural.java:44: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object Interprocedural.returnSourceDirect() at [line 43]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 44]) via { } +Interprocedural.java:48: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object Interprocedural.returnSourceIndirect() at [line 48]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 48]) via { } +Interprocedural.java:58: ERROR: QUANDARY_TAINT_ERROR Error: Other(Interprocedural$Obj Interprocedural.returnSourceViaField() at [line 58]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 58]) via { } +Interprocedural.java:67: ERROR: QUANDARY_TAINT_ERROR Error: Other(void Interprocedural.returnSourceViaParameter1(Interprocedural$Obj) at [line 66]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 67]) via { } Interprocedural.java:77: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 75]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 77]) via { void Interprocedural.returnSourceViaParameter2(Interprocedural$Obj,Interprocedural$Obj) at [line 76] } -Interprocedural.java:92: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 87]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 92]) via { void Interprocedural.returnSourceViaGlobal() at [line 91] } +Interprocedural.java:92: ERROR: QUANDARY_TAINT_ERROR Error: Other(void Interprocedural.returnSourceViaGlobal() at [line 91]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 92]) via { } Interprocedural.java:108: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 108]) -> Other(void Interprocedural.callSinkParam1(Object,Object) at [line 108]) via { } Interprocedural.java:120: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 120]) -> Other(void Interprocedural.callSinkParam2(Object,Object) at [line 120]) via { } Interprocedural.java:133: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 132]) -> Other(void Interprocedural.callSinkOnFieldDirect() at [line 133]) via { } @@ -99,7 +99,7 @@ Interprocedural.java:201: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferT Interprocedural.java:202: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 200]) -> Other(void Interprocedural.callSinkParam2(Object,Object) at [line 202]) via { } Interprocedural.java:210: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 208]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 210]) via { Object Interprocedural.id(Object) at [line 209] } Interprocedural.java:217: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 214]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 217]) via { Object Interprocedural.id(Object) at [line 215], Object Interprocedural.id(Object) at [line 216] } -Interprocedural.java:228: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 223]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 228]) via { Object Interprocedural.returnSourceConditional(boolean) at [line 228] } +Interprocedural.java:228: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object Interprocedural.returnSourceConditional(boolean) at [line 228]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 228]) via { } Interprocedural.java:239: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 237]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 239]) via { } Interprocedural.java:251: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 251]) -> Other(void Interprocedural.callSinkVariadic(java.lang.Object[]) at [line 251]) via { } Interprocedural.java:262: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 260]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 262]) via { }