From 2876f5070390bbdcc62b05cf5eb4b3b7617e2377 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Wed, 16 Aug 2017 16:45:46 -0700 Subject: [PATCH] [quandary] popen as sink Summary: We don't want to miss shell execs via popen. Reviewed By: the-st0rm Differential Revision: D5642788 fbshipit-source-id: 8b20dc1 --- infer/src/quandary/ClangTrace.ml | 10 ++++++---- infer/tests/codetoanalyze/cpp/quandary/execs.cpp | 9 +++++++++ infer/tests/codetoanalyze/cpp/quandary/issues.exp | 1 + 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/infer/src/quandary/ClangTrace.ml b/infer/src/quandary/ClangTrace.ml index 0ab315f6f..6ac095b6d 100644 --- a/infer/src/quandary/ClangTrace.ml +++ b/infer/src/quandary/ClangTrace.ml @@ -182,18 +182,20 @@ module SinkKind = struct | Typ.Procname.ObjC_Cpp cpp_name -> ( match Typ.Procname.get_method pname with | "operator[]" when is_buffer_class cpp_name - -> taint_nth 1 BufferAccess actuals + -> taint_nth 1 BufferAccess actuals | _ - -> get_external_sink pname actuals ) + -> get_external_sink pname actuals ) | Typ.Procname.C _ when Typ.Procname.equal pname BuiltinDecl.__array_access - -> taint_all BufferAccess actuals + -> taint_all BufferAccess actuals | Typ.Procname.C _ when Typ.Procname.equal pname BuiltinDecl.__set_array_length -> (* called when creating a stack-allocated array *) - taint_nth 1 Allocation actuals + taint_nth 1 Allocation actuals | Typ.Procname.C _ -> ( match Typ.Procname.to_string pname with | "execl" | "execlp" | "execle" | "execv" | "execve" | "execvp" | "system" -> taint_all ShellExec actuals + | "popen" + -> taint_nth 0 ShellExec actuals | "brk" | "calloc" | "malloc" | "realloc" | "sbrk" -> taint_all Allocation actuals | _ diff --git a/infer/tests/codetoanalyze/cpp/quandary/execs.cpp b/infer/tests/codetoanalyze/cpp/quandary/execs.cpp index fdc7cf174..90c835b9f 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/execs.cpp +++ b/infer/tests/codetoanalyze/cpp/quandary/execs.cpp @@ -9,6 +9,7 @@ #include #include +#include #include extern int rand(); @@ -83,6 +84,9 @@ int callExecBad() { return execve(NULL, arrSource, NULL); case 14: return system(stringSource); + case 15: + FILE* f = popen(stringSource, "w"); + return pclose(f); } return 0; } @@ -102,6 +106,11 @@ void sql_on_env_var_bad() { __infer_sql_sink(source, 0); } +void tainted_flag_popen_ok() { + char* tainted = std::getenv("something"); + popen("ls", tainted); // should not warn; +} + class Obj { void endpoint(int i, char c, diff --git a/infer/tests/codetoanalyze/cpp/quandary/issues.exp b/infer/tests/codetoanalyze/cpp/quandary/issues.exp index 29ea2ec3b..ded6da025 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/issues.exp +++ b/infer/tests/codetoanalyze/cpp/quandary/issues.exp @@ -46,6 +46,7 @@ codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 27, QUANDARY_TAINT_ERR codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 29, QUANDARY_TAINT_ERROR, [Return from getenv,Call to execve] codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 31, QUANDARY_TAINT_ERROR, [Return from getenv,Call to execve] codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 33, QUANDARY_TAINT_ERROR, [Return from getenv,Call to system] +codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 35, QUANDARY_TAINT_ERROR, [Return from getenv,Call to popen] codetoanalyze/cpp/quandary/execs.cpp, execs::exec_flag_bad, 0, QUANDARY_TAINT_ERROR, [Return from execs::exec_flag_bad,Call to execl] codetoanalyze/cpp/quandary/execs.cpp, execs::sql_on_env_var_bad, 2, QUANDARY_TAINT_ERROR, [Return from getenv,Call to __infer_sql_sink] codetoanalyze/cpp/quandary/files.cpp, files::read_file_call_exec_bad1, 5, QUANDARY_TAINT_ERROR, [Return from std::basic_istream>_read,Call to execle]