diff --git a/infer/src/IR/Typ.re b/infer/src/IR/Typ.re index 3dd65da34..0fa8e4545 100644 --- a/infer/src/IR/Typ.re +++ b/infer/src/IR/Typ.re @@ -230,6 +230,15 @@ let rec compare t1 t2 => let equal t1 t2 => compare t1 t2 == 0; +/** type comparison that treats T* [] and T** as the same type. Needed for C/C++ */ +let array_sensitive_compare t1 t2 => + switch (t1, t2) { + | (Tptr (Tptr ptr_typ _) _, Tarray (Tptr array_typ _) _) => compare ptr_typ array_typ + | (Tarray (Tptr array_typ _) _, Tptr (Tptr ptr_typ _) _) => compare array_typ ptr_typ + | _ => compare t1 t2 + }; + + /** Pretty print a type declaration. pp_base prints the variable for a declaration, or can be skip to print only the type */ let rec pp_decl pe pp_base f => diff --git a/infer/src/IR/Typ.rei b/infer/src/IR/Typ.rei index aea9475d8..b0c9f332c 100644 --- a/infer/src/IR/Typ.rei +++ b/infer/src/IR/Typ.rei @@ -87,6 +87,10 @@ type t = let compare: t => t => int; +/** type comparison that treats T* [] and T** as the same type. Needed for C/C++ */ +let array_sensitive_compare: t => t => int; + + /** Equality for types. */ let equal: t => t => bool; diff --git a/infer/src/checkers/accessPath.ml b/infer/src/checkers/accessPath.ml index bbd86f101..5bb656af1 100644 --- a/infer/src/checkers/accessPath.ml +++ b/infer/src/checkers/accessPath.ml @@ -28,7 +28,7 @@ let base_compare ((var1, typ1) as base1) ((var2, typ2) as base2) = then 0 else Var.compare var1 var2 - |> next Typ.compare typ1 typ2 + |> next Typ.array_sensitive_compare typ1 typ2 let base_equal base1 base2 = base_compare base1 base2 = 0 diff --git a/infer/src/quandary/TaintAnalysis.ml b/infer/src/quandary/TaintAnalysis.ml index b3cc7de65..3e3249bd9 100644 --- a/infer/src/quandary/TaintAnalysis.ml +++ b/infer/src/quandary/TaintAnalysis.ml @@ -152,11 +152,19 @@ module Make (TaintSpec : TaintSpec.S) = struct let add_sink_to_actual access_tree_acc (sink_param : TraceDomain.Sink.t Sink.parameter) = let actual_exp, actual_typ = IList.nth actuals sink_param.index in match AccessPath.of_exp actual_exp actual_typ ~f_resolve_id with - | Some actual_ap -> + | Some actual_ap_raw -> let actual_ap = - if sink_param.report_reachable - then AccessPath.Abstracted actual_ap - else AccessPath.Exact actual_ap in + let is_array_typ = match actual_typ with + | Typ.Tptr (Tarray _, _) (* T* [] (Java-style) *) + | Tptr (Tptr _, _) (* T** (C/C++ style 1) *) + | Tarray _ (* T[] C/C++ style 2 *) -> + true + | _ -> + false in + (* conisder any sources that are reachable from an array *) + if sink_param.report_reachable || is_array_typ + then AccessPath.Abstracted actual_ap_raw + else AccessPath.Exact actual_ap_raw in begin match access_path_get_node actual_ap access_tree_acc proc_data loc with | Some (actual_trace, _) -> diff --git a/infer/tests/codetoanalyze/cpp/quandary/execs.cpp b/infer/tests/codetoanalyze/cpp/quandary/execs.cpp index c2dad5f02..efda5926f 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/execs.cpp +++ b/infer/tests/codetoanalyze/cpp/quandary/execs.cpp @@ -14,7 +14,7 @@ extern int rand(); namespace execs { -int callAllSinks(const char* stringSource) { +int callAllSinks(const char* stringSource, char ** arrSource) { switch (rand()) { case 1: return execl(NULL, stringSource); @@ -35,29 +35,26 @@ int callAllSinks(const char* stringSource) { return execv(stringSource, NULL); case 9: return execvp(stringSource, NULL); + case 10: + return execv(NULL, arrSource); + case 11: + return execvp(NULL, arrSource); } 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); + char* arrSource[1] = {std::getenv("ENV_VAR")}; + callAllSinks(source, arrSource); } extern char* getenv(const char* var); -void execConstantStringOk() { callAllSinks("something.sh"); } +void execConstantStringOk() { callAllSinks("something.sh", NULL); } void customGetEnvOk() { const char* source = execs::getenv("ENV_VAR"); - callAllSinks(source); + callAllSinks(source, NULL); } } diff --git a/infer/tests/codetoanalyze/cpp/quandary/issues.exp b/infer/tests/codetoanalyze/cpp/quandary/issues.exp index 5f1c50f90..b195912ed 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/issues.exp +++ b/infer/tests/codetoanalyze/cpp/quandary/issues.exp @@ -2,12 +2,14 @@ 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] } +execs.cpp:49: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 47]) -> ShellExec(execl at [line 20]) via { execs::callAllSinks at [line 49] } +execs.cpp:49: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 47]) -> ShellExec(execl at [line 22]) via { execs::callAllSinks at [line 49] } +execs.cpp:49: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 47]) -> ShellExec(execl at [line 25]) via { execs::callAllSinks at [line 49] } +execs.cpp:49: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 47]) -> ShellExec(execle at [line 31]) via { execs::callAllSinks at [line 49] } +execs.cpp:49: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 47]) -> ShellExec(execle at [line 33]) via { execs::callAllSinks at [line 49] } +execs.cpp:49: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 47]) -> ShellExec(execlp at [line 27]) via { execs::callAllSinks at [line 49] } +execs.cpp:49: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 47]) -> ShellExec(execlp at [line 29]) via { execs::callAllSinks at [line 49] } +execs.cpp:49: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 47]) -> ShellExec(execv at [line 35]) via { execs::callAllSinks at [line 49] } +execs.cpp:49: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 47]) -> ShellExec(execvp at [line 37]) via { execs::callAllSinks at [line 49] } +execs.cpp:49: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 48]) -> ShellExec(execv at [line 39]) via { execs::callAllSinks at [line 49] } +execs.cpp:49: ERROR: QUANDARY_TAINT_ERROR Error: EnvironmentVariable(getenv at [line 48]) -> ShellExec(execvp at [line 41]) via { execs::callAllSinks at [line 49] } diff --git a/infer/tests/codetoanalyze/java/quandary/Basics.java b/infer/tests/codetoanalyze/java/quandary/Basics.java index e6e95d52e..96640f3df 100644 --- a/infer/tests/codetoanalyze/java/quandary/Basics.java +++ b/infer/tests/codetoanalyze/java/quandary/Basics.java @@ -160,6 +160,12 @@ public class Basics { InferTaint.inferSensitiveSink(src); } + void arrayWithTaintedContentsBad() { + Object src = InferTaint.inferSecretSource(); + Object[] arr = new Object[] { src }; + InferTaint.inferSensitiveSink(arr); + } + /** should not report on these tests */ void directOk1() { diff --git a/infer/tests/codetoanalyze/java/quandary/Interprocedural.java b/infer/tests/codetoanalyze/java/quandary/Interprocedural.java index d057f914e..afed65ebc 100644 --- a/infer/tests/codetoanalyze/java/quandary/Interprocedural.java +++ b/infer/tests/codetoanalyze/java/quandary/Interprocedural.java @@ -240,7 +240,7 @@ class Interprocedural { InferTaint.inferSensitiveSink(params); } - public static void FN_callSinkVariadicBad() { + public static void callSinkVariadicBad() { callSinkVariadic(null, null, InferTaint.inferSecretSource()); } diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index 790511098..54b749b29 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/infer/tests/codetoanalyze/java/quandary/issues.exp @@ -22,8 +22,9 @@ Basics.java:142: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.infe Basics.java:153: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 150]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 153]) via { } Basics.java:159: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 158]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 159]) via { } Basics.java:160: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 158]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 160]) via { } -Basics.java:203: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 200]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 203]) via { } -Basics.java:212: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 208]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 212]) via { } +Basics.java:166: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 164]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 166]) via { } +Basics.java:209: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 206]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 209]) via { } +Basics.java:218: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 214]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 218]) via { } DynamicDispatch.java:77: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 25]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 77]) via { Object DynamicDispatch$BadInterfaceImpl1.returnSource() at [line 76], Object DynamicDispatch$BadInterfaceImpl2.returnSource() at [line 76] } DynamicDispatch.java:77: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 42]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 77]) via { Object DynamicDispatch$BadInterfaceImpl1.returnSource() at [line 76], Object DynamicDispatch$BadInterfaceImpl2.returnSource() at [line 76] } DynamicDispatch.java:82: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 81]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 30]) via { void DynamicDispatch$BadInterfaceImpl1.callSink(Object) at [line 82] } @@ -100,6 +101,7 @@ Interprocedural.java:203: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferT Interprocedural.java:210: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 207]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 210]) via { Object Interprocedural.id(Object) at [line 208], Object Interprocedural.id(Object) at [line 209] } Interprocedural.java:221: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 216]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 221]) via { Object Interprocedural.returnSourceConditional(boolean) at [line 221] } Interprocedural.java:232: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 230]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 232]) via { } +Interprocedural.java:244: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 244]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 240]) via { void Interprocedural.callSinkVariadic(java.lang.Object[]) at [line 244] } Interprocedural.java:255: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 253]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 255]) via { } LoggingPrivateData.java:18: ERROR: QUANDARY_TAINT_ERROR Error: SharedPreferences(String SharedPreferences.getString(String,String) at [line 18]) -> Logging(int Log.d(String,String) at [line 18]) via { } LoggingPrivateData.java:22: ERROR: QUANDARY_TAINT_ERROR Error: SharedPreferences(String SharedPreferences.getString(String,String) at [line 22]) -> Logging(int Log.d(String,String) at [line 22]) via { }