From 072fe0994f532687a269d1631cf19f217518fd06 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Thu, 13 Oct 2016 18:43:06 -0700 Subject: [PATCH] [quandary] reporting on getenv -> exec flows Differential Revision: D4017588 fbshipit-source-id: ae099f8 --- infer/src/quandary/CppTrace.ml | 21 ++++++- .../tests/codetoanalyze/cpp/quandary/Makefile | 1 + .../codetoanalyze/cpp/quandary/execs.cpp | 63 +++++++++++++++++++ .../codetoanalyze/cpp/quandary/issues.exp | 9 +++ 4 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 infer/tests/codetoanalyze/cpp/quandary/execs.cpp diff --git a/infer/src/quandary/CppTrace.ml b/infer/src/quandary/CppTrace.ml index 530fa578b..2955e1e6c 100644 --- a/infer/src/quandary/CppTrace.ml +++ b/infer/src/quandary/CppTrace.ml @@ -17,12 +17,16 @@ module CppSource = struct module SourceKind = struct type t = | Footprint of AccessPath.t (** source that was read from the environment. *) + | EnvironmentVariable (** source that was read from an environment variable *) | Other (** for testing or uncategorized sources *) let compare sk1 sk2 = match sk1, sk2 with | Footprint ap1, Footprint ap2 -> AccessPath.compare ap1 ap2 | Footprint _, _ -> (-1) | _, Footprint _ -> 1 + | EnvironmentVariable, EnvironmentVariable -> 0 + | EnvironmentVariable, _ -> (-1) + | _, EnvironmentVariable -> 1 | Other, Other -> 0 end @@ -65,6 +69,7 @@ module CppSource = struct | (C _) as pname -> begin match Procname.to_string pname with + | "getenv" -> Some (make EnvironmentVariable site) | "__infer_taint_source" -> Some (make Other site) | _ -> None end @@ -82,6 +87,7 @@ module CppSource = struct let pp_kind fmt (kind : kind) = match kind with | Footprint ap -> F.fprintf fmt "Footprint[%a]" AccessPath.pp ap + | EnvironmentVariable -> F.fprintf fmt "EnvironmentVariable" | Other -> F.fprintf fmt "Other" let pp fmt s = @@ -98,9 +104,13 @@ module CppSink = struct module SinkKind = struct type t = + | ShellExec (** shell exec function *) | Other (** for testing or uncategorized sinks *) let compare snk1 snk2 = match snk1, snk2 with + | ShellExec, ShellExec -> 0 + | ShellExec, _ -> (-1) + | _, ShellExec -> 1 | Other, Other -> 0 end @@ -121,7 +131,11 @@ module CppSink = struct let make kind site = { kind; site; } - let get site _ = + let get site actuals = + let taint_all actuals sink ~report_reachable = + IList.mapi + (fun actual_num _ -> Sink.make_sink_param sink actual_num ~report_reachable) + actuals in match CallSite.pname site with | (Procname.ObjC_Cpp cpp_pname) as pname -> begin @@ -133,6 +147,8 @@ module CppSink = struct | (C _ as pname) -> begin match Procname.to_string pname with + | "execl" | "execlp" | "execle" | "execv" | "execvp" -> + taint_all actuals (make ShellExec site) ~report_reachable:false | "__infer_taint_sink" -> [Sink.make_sink_param (make Other site) 0 ~report_reachable:false] | _ -> @@ -154,6 +170,7 @@ module CppSink = struct compare t1 t2 = 0 let pp_kind fmt (kind : kind) = match kind with + | ShellExec -> F.fprintf fmt "ShellExec" | Other -> F.fprintf fmt "Other" let pp fmt s = @@ -175,6 +192,8 @@ include let open Source in let open Sink in match Source.kind source, Sink.kind sink with + | SourceKind.EnvironmentVariable, SinkKind.ShellExec -> + true | SourceKind.Other, SinkKind.Other -> true | _ -> diff --git a/infer/tests/codetoanalyze/cpp/quandary/Makefile b/infer/tests/codetoanalyze/cpp/quandary/Makefile index 348f343d8..29f03a515 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/Makefile +++ b/infer/tests/codetoanalyze/cpp/quandary/Makefile @@ -14,6 +14,7 @@ INFERPRINT_OPTIONS = --issues-txt FILES = \ basics.cpp \ + execs.cpp \ compile: clang $(OPTIONS) $(FILES) diff --git a/infer/tests/codetoanalyze/cpp/quandary/execs.cpp b/infer/tests/codetoanalyze/cpp/quandary/execs.cpp new file mode 100644 index 000000000..c2dad5f02 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/quandary/execs.cpp @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2016 - 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 int rand(); + +namespace execs { + +int callAllSinks(const char* stringSource) { + switch (rand()) { + case 1: + return execl(NULL, stringSource); + case 2: + return execl(stringSource, NULL); + case 3: + // just one test for varargs; assume we get it right in the other cases + return execl(NULL, NULL, stringSource); + case 4: + return execlp(NULL, stringSource); + case 5: + return execlp(stringSource, NULL); + case 6: + return execle(NULL, stringSource); + case 7: + return execle(stringSource, NULL); + case 8: + return execv(stringSource, NULL); + case 9: + return execvp(stringSource, NULL); + } + return 0; +} + +void callExecBad() { + const char* source = std::getenv("ENV_VAR"); + callAllSinks(source); +} + +// need to treat an array as tainted when its contents are tainted to get these +// two +void FN_envVarFlowsToEnvironment() { + char* execEnv[1] = {std::getenv("ENV_VAR")}; + execv("something.s", execEnv); + execvp("something.sh", execEnv); +} + +extern char* getenv(const char* var); + +void execConstantStringOk() { callAllSinks("something.sh"); } + +void customGetEnvOk() { + const char* source = execs::getenv("ENV_VAR"); + callAllSinks(source); +} +} diff --git a/infer/tests/codetoanalyze/cpp/quandary/issues.exp b/infer/tests/codetoanalyze/cpp/quandary/issues.exp index 1b91685b8..5f1c50f90 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/issues.exp +++ b/infer/tests/codetoanalyze/cpp/quandary/issues.exp @@ -2,3 +2,12 @@ basics.cpp:28: ERROR: QUANDARY_TAINT_ERROR Error: Other(__infer_taint_source at basics.cpp:33: ERROR: QUANDARY_TAINT_ERROR Error: Other(__infer_taint_source at [line 20]) -> Other(__infer_taint_sink at [line 33]) via { basics::returnSource at [line 32] } basics.cpp:38: ERROR: QUANDARY_TAINT_ERROR Error: Other(__infer_taint_source at [line 37]) -> Other(__infer_taint_sink at [line 22]) via { basics::callSink at [line 38] } basics.cpp:44: ERROR: QUANDARY_TAINT_ERROR Error: Other(__infer_taint_source at [line 42]) -> Other(__infer_taint_sink at [line 22]) via { basics::callSink at [line 44], basics::id at [line 43] } +execs.cpp:44: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 43]) -> ShellExec(execl at [line 20]) via { execs::callAllSinks at [line 44] } +execs.cpp:44: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 43]) -> ShellExec(execl at [line 22]) via { execs::callAllSinks at [line 44] } +execs.cpp:44: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 43]) -> ShellExec(execl at [line 25]) via { execs::callAllSinks at [line 44] } +execs.cpp:44: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 43]) -> ShellExec(execle at [line 31]) via { execs::callAllSinks at [line 44] } +execs.cpp:44: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 43]) -> ShellExec(execle at [line 33]) via { execs::callAllSinks at [line 44] } +execs.cpp:44: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 43]) -> ShellExec(execlp at [line 27]) via { execs::callAllSinks at [line 44] } +execs.cpp:44: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 43]) -> ShellExec(execlp at [line 29]) via { execs::callAllSinks at [line 44] } +execs.cpp:44: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 43]) -> ShellExec(execv at [line 35]) via { execs::callAllSinks at [line 44] } +execs.cpp:44: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 43]) -> ShellExec(execvp at [line 37]) via { execs::callAllSinks at [line 44] }