From 97f3af15f385dd798fd28ffe3028ac71daf10bfb Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Wed, 15 Nov 2017 07:57:58 -0800 Subject: [PATCH] [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 | 38 +++++-- infer/src/quandary/QuandaryConfig.ml | 7 +- infer/src/quandary/QuandaryConfig.mli | 2 +- .../codetoanalyze/cpp/quandary/.inferconfig | 7 +- .../codetoanalyze/cpp/quandary/issues.exp | 3 + .../codetoanalyze/cpp/quandary/sanitizers.cpp | 105 ++++++++++++++++++ 6 files changed, 150 insertions(+), 12 deletions(-) create mode 100644 infer/tests/codetoanalyze/cpp/quandary/sanitizers.cpp diff --git a/infer/src/quandary/ClangTrace.ml b/infer/src/quandary/ClangTrace.ml index 82df4f04e..23cf9eb3c 100644 --- a/infer/src/quandary/ClangTrace.ml +++ b/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} -> - QualifiedCppName.Match.of_fuzzy_qual_names [procedure]) + ~f:(fun {QuandaryConfig.Sanitizer.procedure; kind} -> + (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 -> - if QualifiedCppName.Match.match_qualifiers qualifiers qualified_pname then Some All + ~f:(fun (qualifiers, kind) -> + 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) -> - (* assume any sanitizer clears all forms of taint *) + | _, sink_kind when is_sanitized sink_kind sanitizers -> + (* trace is sanitized; don't report *) None | Endpoint (_, typ), (ShellExec | SQL) -> (* remote code execution if the caller of the endpoint doesn't sanitize *) diff --git a/infer/src/quandary/QuandaryConfig.ml b/infer/src/quandary/QuandaryConfig.ml index b5a34244c..488ed5a42 100644 --- a/infer/src/quandary/QuandaryConfig.ml +++ b/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 | _ -> diff --git a/infer/src/quandary/QuandaryConfig.mli b/infer/src/quandary/QuandaryConfig.mli index 3685c0b0a..9c0e4129c 100644 --- a/infer/src/quandary/QuandaryConfig.mli +++ b/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 diff --git a/infer/tests/codetoanalyze/cpp/quandary/.inferconfig b/infer/tests/codetoanalyze/cpp/quandary/.inferconfig index da5bd5d60..381d85d33 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/.inferconfig +++ b/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" diff --git a/infer/tests/codetoanalyze/cpp/quandary/issues.exp b/infer/tests/codetoanalyze/cpp/quandary/issues.exp index b2792d8c5..48f09b5e5 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/issues.exp +++ b/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] diff --git a/infer/tests/codetoanalyze/cpp/quandary/sanitizers.cpp b/infer/tests/codetoanalyze/cpp/quandary/sanitizers.cpp new file mode 100644 index 000000000..093762a41 --- /dev/null +++ b/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 +#include + +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