diff --git a/infer/src/IR/Typ.re b/infer/src/IR/Typ.re index 442bfb72d..4367231b6 100644 --- a/infer/src/IR/Typ.re +++ b/infer/src/IR/Typ.re @@ -261,6 +261,14 @@ module Name = { QualifiedCppName.append_template_args_to_last name args::template_suffix } | JavaClass _ => QualifiedCppName.empty; + let unqualified_name = + fun + | CStruct name + | CUnion name + | ObjcClass name + | ObjcProtocol name => name + | CppClass name _ => name + | JavaClass _ => QualifiedCppName.empty; let name n => switch n { | CStruct _ diff --git a/infer/src/IR/Typ.rei b/infer/src/IR/Typ.rei index 804916262..2b6b7d25c 100644 --- a/infer/src/IR/Typ.rei +++ b/infer/src/IR/Typ.rei @@ -142,6 +142,7 @@ module Name: { /** qualified name of the type, may return nonsense for Java classes */ let qual_name: t => QualifiedCppName.t; + let unqualified_name: t => QualifiedCppName.t; module C: { let from_string: string => t; let from_qual_name: QualifiedCppName.t => t; diff --git a/infer/src/checkers/Source.ml b/infer/src/checkers/Source.ml index 3431127d9..df4dc1e93 100644 --- a/infer/src/checkers/Source.ml +++ b/infer/src/checkers/Source.ml @@ -21,7 +21,7 @@ module type Kind = sig val unknown : t - val get : Typ.Procname.t -> Tenv.t -> t option + val get : Typ.Procname.t -> Tenv.t -> (t * int option) option val get_tainted_formals : Procdesc.t -> Tenv.t -> (Mangled.t * Typ.t * t option) list end @@ -29,13 +29,19 @@ end module type S = sig include TraceElem.S + type spec = + { + source : t; + index : int option; + } + val is_footprint : t -> bool val make_footprint : AccessPath.t -> Procdesc.t -> t val get_footprint_access_path: t -> AccessPath.t option - val get : CallSite.t -> Tenv.t -> t option + val get : CallSite.t -> Tenv.t -> spec option val get_tainted_formals : Procdesc.t -> Tenv.t -> (Mangled.t * Typ.t * t option) list end @@ -58,6 +64,12 @@ module Make (Kind : Kind) = struct site : CallSite.t; } [@@deriving compare] + type spec = + { + source : t; + index : int option; + } + let is_footprint t = match t.kind with | Footprint _ -> true | _ -> false @@ -82,8 +94,11 @@ module Make (Kind : Kind) = struct { site; kind; } let get site tenv = match Kind.get (CallSite.pname site) tenv with - | Some kind -> Some (make kind site) - | None -> None + | Some (kind, index) -> + let source = make kind site in + Some { source; index; } + | None -> + None let get_tainted_formals pdesc tenv = let site = CallSite.make (Procdesc.get_proc_name pdesc) (Procdesc.get_loc pdesc) in @@ -110,6 +125,12 @@ end module Dummy = struct type t = unit [@@deriving compare] + type spec = + { + source : t; + index : int option; + } + let call_site _ = CallSite.dummy let kind t = t diff --git a/infer/src/checkers/Source.mli b/infer/src/checkers/Source.mli index bbbb6347e..f178bf830 100644 --- a/infer/src/checkers/Source.mli +++ b/infer/src/checkers/Source.mli @@ -19,7 +19,7 @@ module type Kind = sig val unknown : t (** return Some (kind) if the procedure is a taint source, None otherwise *) - val get : Typ.Procname.t -> Tenv.t -> t option + val get : Typ.Procname.t -> Tenv.t -> (t * int option) option (** return each formal of the function paired with either Some(kind) if the formal is a taint source, or None if the formal is not a taint source *) @@ -29,6 +29,14 @@ end module type S = sig include TraceElem.S + type spec = + { + source : t; + (** type of the returned source *) + index : int option; + (** index of the returned source if Some; return value if None *) + } + (** return true if the current source is a footprint source *) val is_footprint : t -> bool @@ -38,8 +46,8 @@ module type S = sig (** return Some(access path) if the current source is a footprint source, None otherwise *) val get_footprint_access_path: t -> AccessPath.t option - (** return Some (source) if the call site is a taint source, None otherwise *) - val get : CallSite.t -> Tenv.t -> t option + (** return Some (taint spec) if the call site is a taint source, None otherwise *) + val get : CallSite.t -> Tenv.t -> spec option (** return each formal of the function paired with either Some(source) if the formal is a taint source, or None if the formal is not a taint source *) diff --git a/infer/src/quandary/ClangTrace.ml b/infer/src/quandary/ClangTrace.ml index 19868cb1a..d799b4198 100644 --- a/infer/src/quandary/ClangTrace.ml +++ b/infer/src/quandary/ClangTrace.ml @@ -15,6 +15,7 @@ module L = Logging module SourceKind = struct type t = | EnvironmentVariable (** source that was read from an environment variable *) + | File (** source that was read from a file *) | Other (** for testing or uncategorized sources *) | Unknown [@@deriving compare] @@ -23,6 +24,7 @@ module SourceKind = struct let of_string = function | "EnvironmentVariable" -> EnvironmentVariable + | "File" -> File | _ -> Other let external_sources = @@ -32,23 +34,38 @@ module SourceKind = struct (QuandaryConfig.Source.of_json Config.quandary_sources) (* return Some(source kind) if [procedure_name] is in the list of externally specified sources *) - let get_external_source pname = - let qualified_pname = Typ.Procname.get_qualifiers pname in + let get_external_source qualified_pname = + let return = None in List.find_map ~f:(fun (qualifiers, kind) -> if QualifiedCppName.Match.match_qualifiers qualifiers qualified_pname - then Some (of_string kind) + then Some (of_string kind, return) else None) external_sources - let get pname _ = match pname with - | Typ.Procname.ObjC_Cpp _ -> - get_external_source pname + let get pname _ = + let return = None in + match pname with + | Typ.Procname.ObjC_Cpp cpp_name -> + let qualified_pname = Typ.Procname.get_qualifiers pname in + begin + match + (QualifiedCppName.to_list + (Typ.Name.unqualified_name (Typ.Procname.objc_cpp_get_class_type_name cpp_name))), + Typ.Procname.get_method pname with + | ["std"; ("basic_istream" | "basic_iostream")], + ("getline" | "read" | "readsome" | "operator>>") -> + Some (File, Some 1) + | _ -> + get_external_source qualified_pname + end | Typ.Procname.C _ -> begin match Typ.Procname.to_string pname with - | "getenv" -> Some EnvironmentVariable - | _ -> get_external_source pname + | "getenv" -> + Some (EnvironmentVariable, return) + | _ -> + get_external_source (Typ.Procname.get_qualifiers pname) end | Typ.Procname.Block _ -> None @@ -60,10 +77,13 @@ module SourceKind = struct let get_tainted_formals pdesc _ = Source.all_formals_untainted pdesc - let pp fmt = function - | EnvironmentVariable -> F.fprintf fmt "EnvironmentVariable" - | Other -> F.fprintf fmt "Other" - | Unknown -> F.fprintf fmt "Unknown" + let pp fmt kind = + F.fprintf fmt + (match kind with + | EnvironmentVariable -> "EnvironmentVariable" + | File -> "File" + | Other -> "Other" + | Unknown -> "Unknown") end module CppSource = Source.Make(SourceKind) @@ -145,7 +165,9 @@ include let should_report source sink = match Source.kind source, Sink.kind sink with - | EnvironmentVariable, ShellExec -> + | EnvironmentVariable, ShellExec + | File, ShellExec -> + (* untrusted data flowing to exec *) true | Other, Other -> true diff --git a/infer/src/quandary/JavaTrace.ml b/infer/src/quandary/JavaTrace.ml index 150252696..4ecee16af 100644 --- a/infer/src/quandary/JavaTrace.ml +++ b/infer/src/quandary/JavaTrace.ml @@ -34,31 +34,33 @@ module SourceKind = struct ~f:(fun { QuandaryConfig.Source.procedure; kind; } -> Str.regexp procedure, kind) (QuandaryConfig.Source.of_json Config.quandary_sources) - let get pname tenv = match pname with + let get pname tenv = + let return = None in + match pname with | Typ.Procname.Java pname -> begin match Typ.Procname.java_get_class_name pname, Typ.Procname.java_get_method pname with | "android.location.Location", ("getAltitude" | "getBearing" | "getLatitude" | "getLongitude" | "getSpeed") -> - Some PrivateData + Some (PrivateData, return) | "android.telephony.TelephonyManager", ("getDeviceId" | "getLine1Number" | "getSimSerialNumber" | "getSubscriberId" | "getVoiceMailNumber") -> - Some PrivateData + Some (PrivateData, return) | "com.facebook.infer.builtins.InferTaint", "inferSecretSource" -> - Some Other + Some (Other, return) | class_name, method_name -> let taint_matching_supertype typename _ = match Typ.Name.name typename, method_name with | "android.app.Activity", "getIntent" -> - Some Intent + Some (Intent, return) | "android.content.Intent", "getStringExtra" -> - Some Intent + Some (Intent, return) | "android.content.SharedPreferences", "getString" -> - Some PrivateData + Some (PrivateData, return) | _ -> None in let kind_opt = @@ -75,7 +77,7 @@ module SourceKind = struct List.find_map ~f:(fun (procedure_regex, kind) -> if Str.string_match procedure_regex procedure 0 - then Some (of_string kind) + then Some (of_string kind, return) else None) external_sources end diff --git a/infer/src/quandary/TaintAnalysis.ml b/infer/src/quandary/TaintAnalysis.ml index aa97e6188..e2fe0f1c4 100644 --- a/infer/src/quandary/TaintAnalysis.ml +++ b/infer/src/quandary/TaintAnalysis.ml @@ -96,11 +96,22 @@ module Make (TaintSpecification : TaintSpec.S) = struct | AccessPath access_path -> exp_get_node_ ~abstracted access_path access_tree proc_data | _ -> None - let add_source source ret_base access_tree = + let add_return_source source ret_base access_tree = let trace = TraceDomain.of_source source in let id_ap = AccessPath.Exact (ret_base, []) in TaintDomain.add_trace id_ap trace access_tree + let add_actual_source source index actuals access_tree proc_data = + match List.nth_exn actuals index with + | HilExp.AccessPath actual_ap_raw -> + let actual_ap = AccessPath.Exact actual_ap_raw in + let trace = access_path_get_trace actual_ap access_tree proc_data in + TaintDomain.add_trace actual_ap (TraceDomain.add_source source trace) access_tree + | _ -> + access_tree + | exception (Failure _) -> + failwithf "Bad source specification: index %d out of bounds" index + let endpoints = String.Set.of_list (QuandaryConfig.Endpoint.of_json Config.quandary_endpoints) let is_endpoint source = @@ -368,9 +379,11 @@ module Make (TaintSpecification : TaintSpec.S) = struct let source = TraceDomain.Source.get call_site proc_data.tenv in let astate_with_source = match source, ret_opt with - | Some source, Some ret_base -> - add_source source ret_base astate_with_sink - | Some source, None -> + | Some { TraceDomain.Source.source; index=None; }, Some ret_base -> + add_return_source source ret_base astate_with_sink + | Some { TraceDomain.Source.source; index=Some index; }, _ -> + add_actual_source source index actuals astate_with_sink proc_data + | Some { TraceDomain.Source.source; index=None; }, None -> let warn_invalid_source () = L.stderr "Warning: %a is marked as a source, but has no return value" @@ -384,7 +397,7 @@ module Make (TaintSpecification : TaintSpec.S) = struct match List.last actuals with | Some (HilExp.AccessPath ((Var.ProgramVar pvar, _) as ret_base, [])) when Pvar.is_frontend_tmp pvar -> - add_source source ret_base astate_with_sink + add_return_source source ret_base astate_with_sink | _ -> warn_invalid_source () else diff --git a/infer/src/unit/TaintTests.ml b/infer/src/unit/TaintTests.ml index a39377fc2..19a15d5b0 100644 --- a/infer/src/unit/TaintTests.ml +++ b/infer/src/unit/TaintTests.ml @@ -20,7 +20,7 @@ module MockTrace = Trace.Make(struct let get pname _ = if String.is_prefix ~prefix:"SOURCE" (Typ.Procname.to_string pname) - then Some (CallSite.make pname Location.dummy) + then Some (CallSite.make pname Location.dummy, None) else None let get_tainted_formals _ _ = diff --git a/infer/tests/codetoanalyze/cpp/quandary/Makefile b/infer/tests/codetoanalyze/cpp/quandary/Makefile index 4fc50bc8c..085246a87 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/Makefile +++ b/infer/tests/codetoanalyze/cpp/quandary/Makefile @@ -13,8 +13,6 @@ CLANG_OPTIONS = -x c++ -std=c++11 -nostdinc++ -isystem$(ROOT_DIR) -isystem$(CLAN INFER_OPTIONS = --ml-buckets cpp --no-filtering --debug-exceptions --project-root $(TESTS_DIR) --no-failures-allowed INFERPRINT_OPTIONS = --issues-tests -SOURCES = \ - basics.cpp \ - execs.cpp \ +SOURCES = $(wildcard *.cpp) include $(TESTS_DIR)/clang.make diff --git a/infer/tests/codetoanalyze/cpp/quandary/execs.cpp b/infer/tests/codetoanalyze/cpp/quandary/execs.cpp index 3bb7ecd89..d1ceca741 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/execs.cpp +++ b/infer/tests/codetoanalyze/cpp/quandary/execs.cpp @@ -78,7 +78,6 @@ int callExecBad() { extern char* getenv(const char* var); void execConstantStringOk() { callAllSinks("something.sh", NULL); } - void customGetEnvOk() { const char* source = execs::getenv("ENV_VAR"); return execl(NULL, source); diff --git a/infer/tests/codetoanalyze/cpp/quandary/files.cpp b/infer/tests/codetoanalyze/cpp/quandary/files.cpp new file mode 100644 index 000000000..e82f5ec26 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/quandary/files.cpp @@ -0,0 +1,64 @@ +/* + * 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 +#include + +namespace files { + +void read_file_call_exec_bad1(int length) { + std::ifstream is("test.txt", std::ifstream::binary); + if (is) { + char* buffer = new char[length]; + is.read(buffer, length); + execle(buffer, NULL); + is.close(); + } +} + +void read_file_call_exec_bad2(int length) { + std::ifstream is("test.txt", std::ifstream::binary); + if (is) { + char* buffer = new char[length]; + is.readsome(buffer, length); + execle(buffer, NULL); + is.close(); + } +} + +void read_file_call_exec_bad3(int length) { + std::ifstream is("test.txt", std::ifstream::binary); + if (is) { + char* buffer = new char[length]; + is.getline(buffer, length); + execle(buffer, NULL); + is.close(); + } +} + +// have to do matching on C procnames to make this work +void FN_read_file_call_exec_bad5(int length) { + std::ifstream is("test.txt", std::ifstream::binary); + if (is) { + char* buffer = new char[length]; + is >> buffer; + execle(buffer, NULL); + is.close(); + } +} + +// make sure we handle reads via iostreams too +void read_file_call_exec_bad5(std::iostream is, int length) { + if (is) { + char* buffer = new char[length]; + is.getline(buffer, length); + execle(buffer, NULL); + } +} +} diff --git a/infer/tests/codetoanalyze/cpp/quandary/issues.exp b/infer/tests/codetoanalyze/cpp/quandary/issues.exp index d14c41a29..7315d046e 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/issues.exp +++ b/infer/tests/codetoanalyze/cpp/quandary/issues.exp @@ -20,3 +20,7 @@ codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 21, QUANDARY_TAINT_ERR codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 23, QUANDARY_TAINT_ERROR, [return from getenv,call to execvp] codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 25, QUANDARY_TAINT_ERROR, [return from getenv,call to execv] codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 27, QUANDARY_TAINT_ERROR, [return from getenv,call to execvp] +codetoanalyze/cpp/quandary/files.cpp, files::read_file_call_exec_bad1, 5, QUANDARY_TAINT_ERROR, [return from std::basic_istream>_read,call to execle] +codetoanalyze/cpp/quandary/files.cpp, files::read_file_call_exec_bad2, 5, QUANDARY_TAINT_ERROR, [return from std::basic_istream>_readsome,call to execle] +codetoanalyze/cpp/quandary/files.cpp, files::read_file_call_exec_bad3, 5, QUANDARY_TAINT_ERROR, [return from std::basic_istream>_getline,call to execle] +codetoanalyze/cpp/quandary/files.cpp, files::read_file_call_exec_bad5, 4, QUANDARY_TAINT_ERROR, [return from std::basic_istream>_getline,call to execle]