[quandary] support `StringBuilder`'s and other methods for propagating `String` taint

Summary:
Our default strategy for handling unknown code is to propagate taint from the actuals to the return value.
But for commonly-used methods like `StringBuilder.append` (used every time you do `+` with a string in Java), this doesn't work.
The taint should be propagated to both the receiver and the return value in these cases.

I'm considering a solution where we always propagate taint to the receiver of unknown functions in the future, but I am concerned about the performance.
So let's stick with a few special string cases for now.

Reviewed By: cristianoc

Differential Revision: D4124355

fbshipit-source-id: 5b2a232

infer/src/quandary/JavaTaintAnalysis.ml

@@ -44,9 +44,10 @@ include
       let footprint_trace = Trace.of_source footprint_source in
       QuandarySummary.make_in_out_summary input output (to_summary_trace footprint_trace)
 
-    (* propagate the trace associated with each actual to the actual corresponding to the
-       constructed object *)
-    let propagate_to_constructor site actuals ~propagate_all =
+    (* propagate the trace associated with non-receiver actual to the receiver actual. also useful
+       for propagating taint from constructor actuals to the constructed object (which, like the
+       receiver, is also the first argument) *)
+    let propagate_to_receiver site actuals ~propagate_all =
       match actuals with
       | [] ->
           failwithf
@@ -83,7 +84,12 @@ include
                   Procname.java_get_method java_pname,
                   ret_typ_opt with
             | _ when Procname.is_constructor pname ->
-                propagate_to_constructor site actuals ~propagate_all:true
+                propagate_to_receiver site actuals ~propagate_all:true
+            | ("java.lang.StringBuffer" | "java.lang.StringBuilder" | "java.util.Formatter"), _,
+              Some ret_typ
+              when not (Procname.java_is_static pname) ->
+                (propagate_actuals_to_return site ret_typ actuals ~propagate_all:true) @
+                (propagate_to_receiver site actuals ~propagate_all:true)
             | _, _, Some ret_typ ->
                 propagate_actuals_to_return site ret_typ actuals ~propagate_all:true
             | _ ->

infer/src/quandary/TaintAnalysis.ml

@@ -263,7 +263,10 @@ module Make (TaintSpec : TaintSpec.S) = struct
         let caller_ap_trace_opt =
           match in_out_summary.output with
           | Out_return ret_ap ->
-              Some (apply_return ret_ap ret_id, TraceDomain.initial)
+              let caller_ret_ap = apply_return ret_ap ret_id in
+              let ret_trace =
+                access_path_get_trace caller_ret_ap access_tree proc_data callee_loc in
+              Some (caller_ret_ap, ret_trace)
           | Out_formal (formal_num, formal_ap) ->
               get_actual_ap_trace formal_num formal_ap access_tree
           | Out_global global_ap ->

infer/tests/codetoanalyze/java/quandary/Strings.java

@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2015 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+//package codetoanalyze.java.quandary;
+
+import java.util.Formatter;
+
+import com.facebook.infer.builtins.InferTaint;
+
+/** a lot of tainted values are strings, so propagation through StringBuilder's and the like is very
+  * important. */
+
+public class Strings {
+
+  void viaStringBuilderSugarBad() {
+    Object source = InferTaint.inferSecretSource();
+    InferTaint.inferSensitiveSink(source + "");
+  }
+
+  void viaStringBuilderBad() {
+    Object source = InferTaint.inferSecretSource();
+    StringBuilder builder = new StringBuilder();
+    InferTaint.inferSensitiveSink(builder.append(source).append("").toString());
+  }
+
+  void viaStringBuilderIgnoreReturnBad() {
+    Object source = InferTaint.inferSecretSource();
+    StringBuilder builder = new StringBuilder();
+    // builder should be tainted after this call even though we ignore the return value
+    builder.append(source);
+    InferTaint.inferSensitiveSink(builder.toString());
+  }
+
+  void viaStringBufferBad() {
+    Object source = InferTaint.inferSecretSource();
+    StringBuffer buffer = new StringBuffer();
+    InferTaint.inferSensitiveSink(buffer.append("").append(source).toString());
+  }
+
+  void viaStringBufferIgnoreReturnBad() {
+    Object source = InferTaint.inferSecretSource();
+    StringBuffer buffer = new StringBuffer();
+    buffer.append(source);
+    InferTaint.inferSensitiveSink(buffer.toString());
+  }
+
+  void viaFormatterBad() {
+    Object source = InferTaint.inferSecretSource();
+    Formatter formatter = new Formatter();
+    InferTaint.inferSensitiveSink(formatter.format("%s", source).toString());
+  }
+
+  void viaFormatterIgnoreReturnBad() {
+    Object source = InferTaint.inferSecretSource();
+    Formatter formatter = new Formatter();
+    formatter.format("%s", source);
+    InferTaint.inferSensitiveSink(formatter.toString());
+  }
+
+}

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

@@ -146,5 +146,12 @@ LoggingPrivateData.java:60: ERROR: QUANDARY_TAINT_ERROR Error: PrivateData(float
 Recursion.java:26: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 26]) -> Other(void Recursion.callSinkThenDiverge(Object) at [line 26]) 
 Recursion.java:36: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 36]) -> Other(void Recursion.safeRecursionCallSink(int,Object) at [line 36]) 
 Recursion.java:42: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 42]) -> Other(void Recursion.recursionBad(int,Object) at [line 42]) 
+Strings.java:23: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 22]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 23]) (via { StringBuilder StringBuilder.append(Object) at [line 23], StringBuilder StringBuilder.append(String) at [line 23], String StringBuilder.toString() at [line 23] })
+Strings.java:29: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 27]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 29]) (via { StringBuilder StringBuilder.append(Object) at [line 29], StringBuilder StringBuilder.append(String) at [line 29], String StringBuilder.toString() at [line 29] })
+Strings.java:37: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 33]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 37]) (via { StringBuilder StringBuilder.append(Object) at [line 36], String StringBuilder.toString() at [line 37] })
+Strings.java:43: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 41]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 43]) (via { StringBuffer StringBuffer.append(Object) at [line 43], StringBuffer StringBuffer.append(String) at [line 43], String StringBuffer.toString() at [line 43] })
+Strings.java:50: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 47]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 50]) (via { StringBuffer StringBuffer.append(Object) at [line 49], String StringBuffer.toString() at [line 50] })
+Strings.java:56: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 54]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 56]) (via { Formatter Formatter.format(String,java.lang.Object[]) at [line 56], String Formatter.toString() at [line 56] })
+Strings.java:63: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 60]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 63]) (via { Formatter Formatter.format(String,java.lang.Object[]) at [line 62], String Formatter.toString() at [line 63] })
 UnknownCode.java:25: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 23]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 25]) (via { Object UnknownCode.id(Object) at [line 24] })
 UnknownCode.java:32: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 29]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 32]) (via { String.<init>(String) at [line 31] })