From b57aa90d7d31088b336d2d8c2f73c1e763de873f Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Thu, 15 Mar 2018 17:38:16 -0700 Subject: [PATCH] [quandary] don't crash if JSON source/sink is invalid procedure name Summary: At the moment, Java and Clang sources/sinks live in the same inferconfig entry. If we try to parse a Java procedure that happens to be an invalid Clang qualified name (e.g., `MyClass.`), parsing will crash. As a temporary fix, throw an exception and catch it instead. In the future, we can avoid this by requiring that JSON source/sink specifications to indicate the language. Reviewed By: mbouaziz Differential Revision: D7291880 fbshipit-source-id: f8f4502 --- infer/src/IR/QualifiedCppName.ml | 17 +++++++++++------ infer/src/IR/QualifiedCppName.mli | 2 ++ infer/src/quandary/ClangTrace.ml | 18 ++++++++++++++---- .../codetoanalyze/java/quandary/.inferconfig | 5 +++++ .../java/quandary/ExternalSpecs.java | 12 ++++++++++++ .../codetoanalyze/java/quandary/issues.exp | 1 + 6 files changed, 45 insertions(+), 10 deletions(-) diff --git a/infer/src/IR/QualifiedCppName.ml b/infer/src/IR/QualifiedCppName.ml index cc6e411c0..6b7c1d05f 100644 --- a/infer/src/IR/QualifiedCppName.ml +++ b/infer/src/IR/QualifiedCppName.ml @@ -8,8 +8,11 @@ *) open! IStd +module F = Format module L = Logging +exception ParseError of string + (* internally it uses reversed list to store qualified name, for example: ["get", "shared_ptr", "std"]*) type t = string list [@@deriving compare] @@ -27,13 +30,15 @@ let strip_template_args quals = let append_template_args_to_last quals ~args = match quals with | [last; _] when String.contains last '<' -> - L.(die InternalError) - "expected qualified name without template args, but got %s, the last qualifier of %s" last - (String.concat ~sep:", " quals) + raise + (ParseError + (F.sprintf + "expected qualified name without template args, but got %s, the last qualifier of %s" + last (String.concat ~sep:", " quals))) | last :: rest -> (last ^ args) :: rest | [] -> - L.(die InternalError) "expected non-empty qualified name" + raise (ParseError "expected non-empty qualified name") let to_list = List.rev @@ -51,7 +56,7 @@ let from_field_qualified_name qual_name = | _ :: rest -> rest | _ -> - L.(die InternalError) "expected non-empty qualified name" + raise (ParseError "expected non-empty qualified name") (* define [cpp_separator_regex] here to compute it once *) @@ -93,7 +98,7 @@ module Match = struct List.iter colon_splits ~f:(fun s -> (* Filter out the '<' in operator< and operator<= *) if not (String.is_prefix s ~prefix:"operator<") && String.contains s '<' then - L.(die InternalError) "Unexpected template in fuzzy qualified name %s." qual_name ) ; + raise (ParseError ("Unexpected template in fuzzy qualified name %s." ^ qual_name)) ) ; of_qual_string qual_name diff --git a/infer/src/IR/QualifiedCppName.mli b/infer/src/IR/QualifiedCppName.mli index ee0334c3c..46d721360 100644 --- a/infer/src/IR/QualifiedCppName.mli +++ b/infer/src/IR/QualifiedCppName.mli @@ -9,6 +9,8 @@ open! IStd +exception ParseError of string + type t [@@deriving compare] val empty : t diff --git a/infer/src/quandary/ClangTrace.ml b/infer/src/quandary/ClangTrace.ml index 770b37daa..7469eb56d 100644 --- a/infer/src/quandary/ClangTrace.ml +++ b/infer/src/quandary/ClangTrace.ml @@ -11,6 +11,16 @@ open! IStd module F = Format module L = Logging +let parse_clang_procedure procedure kind index = + try Some (QualifiedCppName.Match.of_fuzzy_qual_names [procedure], kind, index) + with QualifiedCppName.ParseError _ -> + (* Java and Clang sources/sinks live in the same inferconfig entry. If we try to parse a Java + procedure that happens to be an invalid Clang qualified name (e.g., MyClass.), + parsing will crash. In the future, we can avoid this by requiring JSON source/sink + specifications to indicate the language *) + None + + module SourceKind = struct type t = | CommandLineFlag of (Var.t * Typ.desc) (** source that was read from a command line flag *) @@ -40,9 +50,9 @@ module SourceKind = struct let external_sources = - List.map + List.filter_map ~f:(fun {QuandaryConfig.Source.procedure; kind; index} -> - (QualifiedCppName.Match.of_fuzzy_qual_names [procedure], kind, index) ) + parse_clang_procedure procedure kind index ) (QuandaryConfig.Source.of_json Config.quandary_sources) @@ -215,9 +225,9 @@ module SinkKind = struct let external_sinks = - List.map + List.filter_map ~f:(fun {QuandaryConfig.Sink.procedure; kind; index} -> - (QualifiedCppName.Match.of_fuzzy_qual_names [procedure], kind, index) ) + parse_clang_procedure procedure kind index ) (QuandaryConfig.Sink.of_json Config.quandary_sinks) diff --git a/infer/tests/codetoanalyze/java/quandary/.inferconfig b/infer/tests/codetoanalyze/java/quandary/.inferconfig index 3bd7dec6a..055db6a95 100644 --- a/infer/tests/codetoanalyze/java/quandary/.inferconfig +++ b/infer/tests/codetoanalyze/java/quandary/.inferconfig @@ -27,6 +27,11 @@ { "procedure": "codetoanalyze.java.quandary.InterfaceSpec.sink", "kind": "Logging" + }, + { + "procedure": "codetoanalyze.java.quandary.ConstructorSink.", + "kind": "Other", + "index": "0" } ], "quandary-sanitizers": [ diff --git a/infer/tests/codetoanalyze/java/quandary/ExternalSpecs.java b/infer/tests/codetoanalyze/java/quandary/ExternalSpecs.java index e33fb263b..86300403d 100644 --- a/infer/tests/codetoanalyze/java/quandary/ExternalSpecs.java +++ b/infer/tests/codetoanalyze/java/quandary/ExternalSpecs.java @@ -175,3 +175,15 @@ class InterfaceSpecImpl implements InterfaceSpec { } } + +class ConstructorSink { + + // specified as a source in .inferconfig + public ConstructorSink(Object o) { + } + + public static ConstructorSink constructorSinkBad() { + Object source = InferTaint.inferSecretSource(); + return new ConstructorSink(source); + } +} diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index 9eb532f6e..c45a38cc8 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/infer/tests/codetoanalyze/java/quandary/issues.exp @@ -43,6 +43,7 @@ codetoanalyze/java/quandary/Exceptions.java, void Exceptions.sinkInCatchBad2(), codetoanalyze/java/quandary/Exceptions.java, void Exceptions.sinkInFinallyBad1(), 5, QUANDARY_TAINT_ERROR, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0] codetoanalyze/java/quandary/Exceptions.java, void Exceptions.sinkInFinallyBad2(), 6, QUANDARY_TAINT_ERROR, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0] codetoanalyze/java/quandary/Exceptions.java, void Exceptions.sinkInFinallyBad3(), 7, QUANDARY_TAINT_ERROR, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0] +codetoanalyze/java/quandary/ExternalSpecs.java, ConstructorSink ConstructorSink.constructorSinkBad(), 2, QUANDARY_TAINT_ERROR, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to ConstructorSink.(Object) with tainted index 1] codetoanalyze/java/quandary/ExternalSpecs.java, Object ExternalSpecs.missedSanitizerBad(), 3, QUANDARY_TAINT_ERROR, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void InferTaint.inferSensitiveSink(Object) with tainted index 0] codetoanalyze/java/quandary/ExternalSpecs.java, void ExternalSpecs.callExternalSink2Bad1(), 1, LOGGING_PRIVATE_DATA, ERROR, [Return from Object ExternalSpecs.privateDataSource(),Call to void ExternalSpecs.loggingSink2(Object,Object) with tainted index 0] codetoanalyze/java/quandary/ExternalSpecs.java, void ExternalSpecs.callExternalSink2Bad2(), 1, LOGGING_PRIVATE_DATA, ERROR, [Return from Object ExternalSpecs.privateDataSource(),Call to void ExternalSpecs.loggingSink2(Object,Object) with tainted index 1]