[quandary] String concatenation sanitizes class loading

Reviewed By: jeremydubreil

Differential Revision: D12943175

fbshipit-source-id: 9e1c92d46

infer/src/checkers/Sanitizer.ml

@@ -11,7 +11,7 @@ module F = Format
 module type S = sig
   type t [@@deriving compare]
 
-  val get : Typ.Procname.t -> t option
+  val get : Typ.Procname.t -> Tenv.t -> t option
 
   val pp : F.formatter -> t -> unit
 end
@@ -19,7 +19,7 @@ end
 module Dummy = struct
   type t = unit [@@deriving compare]
 
-  let get _ = None
+  let get _ _ = None
 
   let pp _ _ = ()
 end

infer/src/checkers/Sanitizer.mli

@@ -12,7 +12,7 @@ module F = Format
 module type S = sig
   type t [@@deriving compare]
 
-  val get : Typ.Procname.t -> t option
+  val get : Typ.Procname.t -> Tenv.t -> t option
   (** Get the sanitizer that should be applied to the return value of given procedure, if any *)
 
   val pp : F.formatter -> t -> unit

infer/src/quandary/ClangTrace.ml

@@ -415,7 +415,7 @@ module CppSanitizer = struct
       (QuandaryConfig.Sanitizer.of_json Config.quandary_sanitizers)
 
 
-  let get pname =
+  let get pname _tenv =
     let qualified_pname = Typ.Procname.get_qualifiers pname in
     List.find_map
       ~f:(fun (qualifiers, kind) ->

infer/src/quandary/JavaTrace.ml

@@ -484,7 +484,9 @@ end
 module JavaSink = Sink.Make (SinkKind)
 
 module JavaSanitizer = struct
-  type t = All [@@deriving compare]
+  type t = All | StringConcatenation [@@deriving compare]
+
+  let equal = [%compare.equal: t]
 
   let external_sanitizers =
     List.map
@@ -492,22 +494,34 @@ module JavaSanitizer = struct
       (QuandaryConfig.Sanitizer.of_json Config.quandary_sanitizers)
 
 
-  let get = function
+  let get_external_sanitizer class_name method_name =
+    let procedure_string = Printf.sprintf "%s.%s" class_name method_name in
+    List.find_map
+      ~f:(fun procedure_regex ->
+        if Str.string_match procedure_regex procedure_string 0 then Some All else None )
+      external_sanitizers
+
+
+  let get pname tenv =
+    match pname with
     | Typ.Procname.Java java_pname ->
-        let procedure_string =
-          Printf.sprintf "%s.%s"
-            (Typ.Procname.Java.get_class_name java_pname)
-            (Typ.Procname.Java.get_method java_pname)
+        let method_name = Typ.Procname.Java.get_method java_pname in
+        let sanitizer_matching_supertype typename =
+          match (Typ.Name.name typename, method_name) with
+          | "java.lang.StringBuilder", "append" ->
+              Some StringConcatenation
+          | class_name, method_name ->
+              get_external_sanitizer class_name method_name
         in
-        List.find_map
-          ~f:(fun procedure_regex ->
-            if Str.string_match procedure_regex procedure_string 0 then Some All else None )
-          external_sanitizers
+        PatternMatch.supertype_find_map_opt tenv sanitizer_matching_supertype
+          (Typ.Name.Java.from_string (Typ.Procname.Java.get_class_name java_pname))
     | _ ->
         None
 
 
-  let pp fmt = function All -> F.pp_print_string fmt "All"
+  let pp fmt kind =
+    F.pp_print_string fmt
+      (match kind with All -> "All" | StringConcatenation -> "StringConcatenation")
 end
 
 include Trace.Make (struct
@@ -517,8 +531,11 @@ include Trace.Make (struct
 
   let get_report source sink sanitizers =
     match (Source.kind source, Sink.kind sink) with
-    | _ when not (List.is_empty sanitizers) ->
-        (* assume any sanitizer clears all forms of taint *)
+    | _ when List.mem sanitizers Sanitizer.All ~equal:Sanitizer.equal ->
+        (* the All sanitizer clears any form of taint; don't report *)
+        None
+    | _, ClassLoading when List.mem sanitizers Sanitizer.StringConcatenation ~equal:Sanitizer.equal
+      ->
         None
     | (Endpoint _ | Intent | UserControlledString | UserControlledURI), CreateIntent ->
         (* creating Intent from user-congrolled data *)

infer/src/quandary/TaintAnalysis.ml

@@ -717,7 +717,7 @@ module Make (TaintSpecification : TaintSpec.S) = struct
               | None ->
                   astate_with_summary
               | Some ret_base -> (
-                match TraceDomain.Sanitizer.get callee_pname with
+                match TraceDomain.Sanitizer.get callee_pname proc_data.tenv with
                 | Some sanitizer ->
                     let ret_ap = AccessPath.Abs.Exact (ret_base, []) in
                     let ret_trace = access_path_get_trace ret_ap astate_with_summary proc_data in

infer/tests/codetoanalyze/java/quandary/ClassLoading.java

@@ -28,7 +28,7 @@ public class ClassLoading {
     We don't want to report it as we consider that string concatenation
     sanitizes the user-controlled string for class loading.
   */
-  public void FP_clipboardToClassForNameWithConcatenationGood() {
+  public void clipboardToClassForNameWithConcatenationGood() {
     String javaFileName = "blabla." + this.getUserControlledString();
     try {
       Class cls = Class.forName(javaFileName);

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

@@ -26,7 +26,6 @@ codetoanalyze/java/quandary/Basics.java, codetoanalyze.java.quandary.Basics.viaV
 codetoanalyze/java/quandary/Basics.java, codetoanalyze.java.quandary.Basics.viaVarBad3():void, 4, QUANDARY_TAINT_ERROR, no_bucket, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0]
 codetoanalyze/java/quandary/Basics.java, codetoanalyze.java.quandary.Basics.whileBad1(int):void, 3, QUANDARY_TAINT_ERROR, no_bucket, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0]
 codetoanalyze/java/quandary/Basics.java, codetoanalyze.java.quandary.Basics.whileBad2(int):void, 6, QUANDARY_TAINT_ERROR, no_bucket, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0]
-codetoanalyze/java/quandary/ClassLoading.java, codetoanalyze.java.quandary.ClassLoading.FP_clipboardToClassForNameWithConcatenationGood():void, 3, SHELL_INJECTION, no_bucket, ERROR, [Return from CharSequence ClipboardManager.getText() with tainted data return*,Return from String ClassLoading.getUserControlledString(),Call to Class Class.forName(String) with tainted index 0]
 codetoanalyze/java/quandary/ClassLoading.java, codetoanalyze.java.quandary.ClassLoading.clipboardToClassForNameBad():void, 2, SHELL_INJECTION, no_bucket, ERROR, [Return from CharSequence ClipboardManager.getText() with tainted data return*,Return from String ClassLoading.getUserControlledString(),Call to Class Class.forName(String) with tainted index 0]
 codetoanalyze/java/quandary/ContentProviders.java, codetoanalyze.java.quandary.ContentProviders.bulkInsert(android.net.Uri,android.content.ContentValues[]):int, 1, UNTRUSTED_FILE, no_bucket, ERROR, [Return from int ContentProviders.bulkInsert(Uri,android.content.ContentValues[]),Call to File.<init>(String) with tainted index 1]
 codetoanalyze/java/quandary/ContentProviders.java, codetoanalyze.java.quandary.ContentProviders.call(java.lang.String,java.lang.String,android.os.Bundle):android.os.Bundle, 1, UNTRUSTED_FILE, no_bucket, ERROR, [Return from Bundle ContentProviders.call(String,String,Bundle),Call to File.<init>(String) with tainted index 1]