[quandary] support multiple sanitizer kinds in C++

Summary: Just changing ClangTrace to actually look at the different sanitizer kinds.

Reviewed By: jeremydubreil

Differential Revision: D6325086

fbshipit-source-id: 5da236d

infer/src/quandary/ClangTrace.ml

@@ -298,25 +298,32 @@ end
 module CppSink = Sink.Make (SinkKind)
 
 module CppSanitizer = struct
-  type t = All [@@deriving compare]
+  type t =
+    | Escape  (** escaped string to sanitize SQL injection or ShellExec sinks *)
+    | All  (** sanitizes all forms of taint *)
+    [@@deriving compare]
+
+  let equal = [%compare.equal : t]
+
+  let of_string = function "Escape" -> Escape | _ -> All
 
   let external_sanitizers =
     List.map
-      ~f:(fun {QuandaryConfig.Sanitizer.procedure} ->
+      ~f:(fun {QuandaryConfig.Sanitizer.procedure; kind} ->
-        QualifiedCppName.Match.of_fuzzy_qual_names [procedure])
+        (QualifiedCppName.Match.of_fuzzy_qual_names [procedure], of_string kind))
       (QuandaryConfig.Sanitizer.of_json Config.quandary_sanitizers)
 
 
   let get pname =
     let qualified_pname = Typ.Procname.get_qualifiers pname in
     List.find_map
-      ~f:(fun qualifiers ->
+      ~f:(fun (qualifiers, kind) ->
-        if QualifiedCppName.Match.match_qualifiers qualifiers qualified_pname then Some All
+        if QualifiedCppName.Match.match_qualifiers qualifiers qualified_pname then Some kind
         else None)
       external_sanitizers
 
 
-  let pp fmt = function All -> F.fprintf fmt "All"
+  let pp fmt = function Escape -> F.fprintf fmt "Escape" | All -> F.fprintf fmt "All"
 end
 
 include Trace.Make (struct
@@ -324,6 +331,21 @@ include Trace.Make (struct
   module Sink = CppSink
   module Sanitizer = CppSanitizer
 
+  let is_sanitized (sink_kind: SinkKind.t) sanitizers =
+    match sanitizers with
+    | [] ->
+        false
+    | _ when List.mem sanitizers Sanitizer.All ~equal:Sanitizer.equal ->
+        (* the All sanitizer clears any form of taint *)
+        true
+    | _ ->
+      match sink_kind with
+      | ShellExec | SQL ->
+          List.mem sanitizers Sanitizer.Escape ~equal:Sanitizer.equal
+      | _ ->
+          false
+
+
   let get_report source sink sanitizers =
     (* TODO: make this accept structs/objects too, but not primitive types or enumes *)
     (* using this to match custom string wrappers such as folly::StringPiece *)
@@ -333,8 +355,8 @@ include Trace.Make (struct
       || String.is_substring ~substring:"char*" lowercase_typ
     in
     match (Source.kind source, Sink.kind sink) with
-    | _ when not (List.is_empty sanitizers) ->
+    | _, sink_kind when is_sanitized sink_kind sanitizers ->
-        (* assume any sanitizer clears all forms of taint *)
+        (* trace is sanitized; don't report *)
         None
     | Endpoint (_, typ), (ShellExec | SQL) ->
         (* remote code execution if the caller of the endpoint doesn't sanitize *)

infer/src/quandary/QuandaryConfig.ml

@@ -53,14 +53,17 @@ module Sink = struct
 end
 
 module Sanitizer = struct
-  type t = {procedure: string}
+  type t = {procedure: string; kind: string}
 
   let of_json = function
     | `List sinks ->
         let parse_sanitizer json =
           let open Yojson.Basic in
           let procedure = Util.member "procedure" json |> Util.to_string in
-          {procedure}
+          let kind =
+            Util.member "kind" json |> Util.to_string_option |> Option.value ~default:"All"
+          in
+          {procedure; kind}
         in
         List.map ~f:parse_sanitizer sinks
     | _ ->

infer/src/quandary/QuandaryConfig.mli

@@ -24,7 +24,7 @@ module Sink : sig
 end
 
 module Sanitizer : sig
-  type t = {procedure: string}
+  type t = {procedure: string; kind: string}
 
   val of_json : [> `List of Yojson.Basic.json list] -> t list
 end

infer/tests/codetoanalyze/cpp/quandary/.inferconfig

@@ -65,7 +65,12 @@
   ],
   "quandary-sanitizers": [
     {
-      "procedure": "__infer_string_sanitizer"
+      "procedure": "__infer_all_sanitizer",
+      "kind": "All"
+    },
+    {
+      "procedure": "__infer_string_sanitizer",
+      "kind": "Escape"
     },
     {
       "procedure": "basics::Obj::sanitizer1"

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

@@ -75,6 +75,9 @@ codetoanalyze/cpp/quandary/pointers.cpp, pointers::assign_pointer_pass_to_sink_b
 codetoanalyze/cpp/quandary/pointers.cpp, pointers::assign_source_by_reference_bad1, 3, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source with tainted data @val$0*,Return from pointers::assign_source_by_reference,Call to __infer_taint_sink]
 codetoanalyze/cpp/quandary/pointers.cpp, pointers::assign_source_by_reference_bad2, 2, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source with tainted data @val$0*,Return from pointers::assign_source_by_reference,Call to __infer_taint_sink]
 codetoanalyze/cpp/quandary/pointers.cpp, pointers::assign_source_by_reference_bad3, 3, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source with tainted data @val$0*,Return from pointers::assign_source_by_reference with tainted data @val$0*,Return from pointers::call_assign_source_by_reference,Call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/sanitizers.cpp, sanitizers::dead_sanitizer_bad, 3, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/sanitizers.cpp, sanitizers::escape_string_to_all_bad, 3, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/sanitizers.cpp, sanitizers::kill_sanitizer_bad, 4, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to __infer_taint_sink]
 codetoanalyze/cpp/quandary/strings.cpp, strings::append1_bad, 2, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to __infer_taint_sink]
 codetoanalyze/cpp/quandary/strings.cpp, strings::append2_bad, 3, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to __infer_taint_sink]
 codetoanalyze/cpp/quandary/strings.cpp, strings::assign1_bad, 2, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to __infer_taint_sink]

infer/tests/codetoanalyze/cpp/quandary/sanitizers.cpp

@@ -0,0 +1,105 @@
+/*
+ * Copyright (c) 2017 - 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.
+ */
+
+#include <stdlib.h>
+#include <string>
+
+extern std::string __infer_taint_source();
+
+extern void __infer_taint_sink(std::string);
+extern void __infer_sql_sink(std::string);
+
+extern std::string __infer_all_sanitizer(std::string);
+extern std::string __infer_string_sanitizer(std::string);
+
+namespace sanitizers {
+
+void escape_string_to_sql_ok() {
+  auto source = __infer_taint_source();
+  auto sanitized = __infer_string_sanitizer(source);
+  __infer_sql_sink(sanitized);
+}
+
+void escape_string_to_shell_ok() {
+  auto source = __infer_taint_source();
+  auto sanitized = __infer_string_sanitizer(source);
+  system(sanitized.c_str());
+}
+
+void escape_string_to_all_bad() {
+  auto source = __infer_taint_source();
+  auto sanitized = __infer_string_sanitizer(source);
+  __infer_taint_sink(sanitized); // wrong kind of sanitizer; report
+}
+
+void all_to_all_ok() {
+  auto source = __infer_taint_source();
+  auto sanitized = __infer_all_sanitizer(source);
+  __infer_taint_sink(sanitized);
+}
+
+void dead_sanitizer_bad() {
+  auto source = __infer_taint_source();
+  auto sanitized = __infer_all_sanitizer(source);
+  __infer_taint_sink(
+      source); // the sink does not use the sanitized value; report
+}
+
+void kill_sanitizer_bad() {
+  auto source = __infer_taint_source();
+  auto x = __infer_all_sanitizer(source);
+  x = __infer_taint_source();
+  __infer_taint_sink(x);
+}
+
+void double_sanitize_ok() {
+  auto source = __infer_taint_source();
+  auto x = __infer_all_sanitizer(source);
+  auto y = __infer_string_sanitizer(x);
+  __infer_taint_sink(y);
+}
+
+void FN_sanitize_one_branch_bad(bool b) {
+  auto source = __infer_taint_source();
+  std::string x;
+  if (b) {
+    x = source;
+  } else {
+    x = __infer_all_sanitizer(source);
+  }
+  // we'll fail to report here because sanitizers are a powerset domain
+  // ideally they would be an inverted powerset domain, but this is
+  // difficult to pull off because our handling of unknown code implicitly
+  // relies on the assumption that join should be union
+  __infer_taint_sink(x); // should report
+}
+
+void sanitize_both_branches_ok(bool b) {
+  auto source = __infer_taint_source();
+  std::string x;
+  if (b) {
+    x = __infer_all_sanitizer(source);
+  } else {
+    x = __infer_all_sanitizer(source);
+  }
+  __infer_taint_sink(x); // does not report
+}
+
+void different_sanitizer_branches_ok(bool b) {
+  auto source = __infer_taint_source();
+  std::string x;
+  if (b) {
+    x = __infer_all_sanitizer(source);
+  } else {
+    x = __infer_string_sanitizer(source);
+  }
+  __infer_sql_sink(x);
+}
+
+} // namespace sanitizers