From 9a4416f7d4707f2664160ae75459b2a9f6d49d3f Mon Sep 17 00:00:00 2001 From: Mehdi Bouaziz Date: Wed, 7 Nov 2018 04:04:46 -0800 Subject: [PATCH] [quandary] String concatenation sanitizes class loading Reviewed By: jeremydubreil Differential Revision: D12943175 fbshipit-source-id: 9e1c92d46 --- infer/src/checkers/Sanitizer.ml | 4 +- infer/src/checkers/Sanitizer.mli | 2 +- infer/src/quandary/ClangTrace.ml | 2 +- infer/src/quandary/JavaTrace.ml | 43 +++++++++++++------ infer/src/quandary/TaintAnalysis.ml | 2 +- .../java/quandary/ClassLoading.java | 2 +- .../codetoanalyze/java/quandary/issues.exp | 1 - 7 files changed, 36 insertions(+), 20 deletions(-) diff --git a/infer/src/checkers/Sanitizer.ml b/infer/src/checkers/Sanitizer.ml index ef0989ab8..5d93932e7 100644 --- a/infer/src/checkers/Sanitizer.ml +++ b/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 diff --git a/infer/src/checkers/Sanitizer.mli b/infer/src/checkers/Sanitizer.mli index 04cdb2c3d..a48af131f 100644 --- a/infer/src/checkers/Sanitizer.mli +++ b/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 diff --git a/infer/src/quandary/ClangTrace.ml b/infer/src/quandary/ClangTrace.ml index d8b8a6710..429b408bf 100644 --- a/infer/src/quandary/ClangTrace.ml +++ b/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) -> diff --git a/infer/src/quandary/JavaTrace.ml b/infer/src/quandary/JavaTrace.ml index 5898784f8..ee508c77c 100644 --- a/infer/src/quandary/JavaTrace.ml +++ b/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 *) diff --git a/infer/src/quandary/TaintAnalysis.ml b/infer/src/quandary/TaintAnalysis.ml index 606a55f64..b91111207 100644 --- a/infer/src/quandary/TaintAnalysis.ml +++ b/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 diff --git a/infer/tests/codetoanalyze/java/quandary/ClassLoading.java b/infer/tests/codetoanalyze/java/quandary/ClassLoading.java index 1079e1f11..7d9ee9165 100644 --- a/infer/tests/codetoanalyze/java/quandary/ClassLoading.java +++ b/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); diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index ff3659320..7f8df0761 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/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.(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.(String) with tainted index 1]