[quandary] apply summary for sinks

Summary:
A function can both be a sink and propagate source info, but we currently ignore the summary for any function that is also a sink.
This will cause us to under-report for (e.g.) `src1 = source(); src2 = strcpy(dest, src1); exec(src2)`.
This is both a potential buffer overflow and a potential shell injection, but we won't report the second issue.

Reviewed By: jberdine

Differential Revision: D5676167

fbshipit-source-id: 232ab2f

infer/src/quandary/TaintAnalysis.ml

@@ -506,16 +506,14 @@ module Make (TaintSpecification : TaintSpec.S) = struct
           in
           let analyze_call astate_acc callee_pname =
             let call_site = CallSite.make callee_pname callee_loc in
-            let sink =
-              if List.is_empty actuals then None
-              else TraceDomain.Sink.get call_site actuals proc_data.ProcData.tenv
-            in
             let astate_with_sink =
-              match sink with
+              if List.is_empty actuals then astate
-              | Some sink
+              else
-               -> add_sink sink actuals astate proc_data call_site
+                match TraceDomain.Sink.get call_site actuals proc_data.ProcData.tenv with
-              | None
+                | Some sink
-               -> astate
+                 -> add_sink sink actuals astate proc_data call_site
+                | None
+                 -> astate
             in
             let source = TraceDomain.Source.get call_site actuals proc_data.tenv in
             let astate_with_source =
@@ -530,8 +528,8 @@ module Make (TaintSpecification : TaintSpec.S) = struct
                -> astate_with_sink
             in
             let astate_with_summary =
-              if Option.is_some source || Option.is_some sink then
+              if Option.is_some source then
-                (* don't use a summary for a procedure that is a direct source or sink *)
+                (* don't use a summary for a procedure that is a direct source *)
                 astate_with_source
               else
                 match Summary.read_summary proc_data.pdesc callee_pname with

infer/tests/codetoanalyze/java/quandary/.inferconfig

@@ -14,6 +14,10 @@
     {
       "procedure": "codetoanalyze.java.quandary.ExternalSpecs.loggingSink2",
       "kind": "Logging"
+    },
+    {
+      "procedure": "codetoanalyze.java.quandary.ExternalSpecs.sinkThatPropagates",
+      "kind": "Logging"
     }
   ],
   "quandary-sanitizers": [

infer/tests/codetoanalyze/java/quandary/ExternalSpecs.java

@@ -114,4 +114,14 @@ public class ExternalSpecs {
     loggingSink1(o);
   }
 
+  public static Object sinkThatPropagates(Object o) {
+    return o;
+  }
+
+  void callSinkThatPropagatesBad() {
+    Object source = InferTaint.inferSecretSource();
+    Object sourceAgain = sinkThatPropagates(source); // should report
+    loggingSink1(null, sourceAgain); // should report here too
+  }
+
 }

infer/tests/codetoanalyze/java/quandary/Intents.java

@@ -196,10 +196,9 @@ public class Intents {
     String extra = taintedIntent.getStringExtra("foo");
 
     Intent newIntent1 = new Intent();
-    mActivity.startActivity(newIntent1.setData(Uri.parse(extra))); // should report
+    newIntent1.setData(Uri.parse(extra)); // should report
     Intent newIntent2 = new Intent();
-    newIntent2.setData(Uri.parse(extra));
+    newIntent2.setData(Uri.parse(extra)); // should report
-    mActivity.startActivity(newIntent2); // should report
   }
 
   void extraToExtraOk() {

infer/tests/codetoanalyze/java/quandary/issues.exp

@@ -63,6 +63,8 @@ codetoanalyze/java/quandary/ExternalSpecs.java, Object ExternalSpecs.missedSanit
 codetoanalyze/java/quandary/ExternalSpecs.java, void ExternalSpecs.callExternalSink2Bad1(), 1, QUANDARY_TAINT_ERROR, [Return from Object ExternalSpecs.privateDataSource(),Call to void ExternalSpecs.loggingSink2(Object,Object)]
 codetoanalyze/java/quandary/ExternalSpecs.java, void ExternalSpecs.callExternalSink2Bad2(), 1, QUANDARY_TAINT_ERROR, [Return from Object ExternalSpecs.privateDataSource(),Call to void ExternalSpecs.loggingSink2(Object,Object)]
 codetoanalyze/java/quandary/ExternalSpecs.java, void ExternalSpecs.callExternalSinkBad(), 1, QUANDARY_TAINT_ERROR, [Return from Object ExternalSpecs.privateDataSource(),Call to void ExternalSpecs.loggingSink1(Object,Object)]
+codetoanalyze/java/quandary/ExternalSpecs.java, void ExternalSpecs.callSinkThatPropagatesBad(), 2, QUANDARY_TAINT_ERROR, [Return from Object InferTaint.inferSecretSource(),Call to Object ExternalSpecs.sinkThatPropagates(Object)]
+codetoanalyze/java/quandary/ExternalSpecs.java, void ExternalSpecs.callSinkThatPropagatesBad(), 3, QUANDARY_TAINT_ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void ExternalSpecs.loggingSink1(Object,Object)]
 codetoanalyze/java/quandary/ExternalSpecs.java, void ExternalSpecs.logExternalSourceBad(), 1, QUANDARY_TAINT_ERROR, [Return from Object ExternalSpecs.privateDataSource(),Call to int Log.e(String,String)]
 codetoanalyze/java/quandary/Fields.java, void Fields.instanceFieldBad(), 2, QUANDARY_TAINT_ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Fields.java, void Fields.staticFieldBad(), 2, QUANDARY_TAINT_ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object)]