From 32675a7b02adef2cd6bce1ef9d57320bc9db3bbd Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Wed, 13 Dec 2017 14:17:55 -0800 Subject: [PATCH] [quandary] improve curl_easy_setopt sink Reviewed By: jeremydubreil Differential Revision: D6557133 fbshipit-source-id: 4df7b49 --- infer/src/IR/HilExp.ml | 26 +++++++++++++++++++ infer/src/IR/HilExp.mli | 2 ++ infer/src/quandary/ClangTrace.ml | 20 +++++++++++--- .../codetoanalyze/cpp/quandary/endpoints.cpp | 16 ++++++++++++ .../codetoanalyze/cpp/quandary/issues.exp | 2 ++ 5 files changed, 63 insertions(+), 3 deletions(-) diff --git a/infer/src/IR/HilExp.ml b/infer/src/IR/HilExp.ml index f49dff663..5eb2352ed 100644 --- a/infer/src/IR/HilExp.ml +++ b/infer/src/IR/HilExp.ml @@ -171,3 +171,29 @@ let of_sil ~include_array_indexes ~f_resolve_id exp typ = let is_null_literal = function Constant Cint n -> IntLit.isnull n | _ -> false + +let rec eval_arithmetic_binop op e1 e2 = + match (eval e1, eval e2) with + | Some Const.Cint i1, Some Const.Cint i2 -> + Some (Const.Cint (op i1 i2)) + | _ -> + None + + +and eval = function + | Constant c -> + Some c + | BinaryOperator (Binop.Div, e1, e2) -> ( + try eval_arithmetic_binop IntLit.div e1 e2 with Division_by_zero -> None ) + | BinaryOperator (Binop.MinusA, e1, e2) -> + eval_arithmetic_binop IntLit.sub e1 e2 + | BinaryOperator (Binop.Mod, e1, e2) -> + eval_arithmetic_binop IntLit.rem e1 e2 + | BinaryOperator (Binop.Mult, e1, e2) -> + eval_arithmetic_binop IntLit.mul e1 e2 + | BinaryOperator (Binop.PlusA, e1, e2) -> + eval_arithmetic_binop IntLit.add e1 e2 + | _ -> + (* TODO: handle bitshifting cases, port eval_binop from RacerD.ml *) + None + diff --git a/infer/src/IR/HilExp.mli b/infer/src/IR/HilExp.mli index 6f1b9d90a..5fd7f376f 100644 --- a/infer/src/IR/HilExp.mli +++ b/infer/src/IR/HilExp.mli @@ -38,3 +38,5 @@ val get_access_paths : t -> AccessPath.t list used more than once. *) val is_null_literal : t -> bool + +val eval : t -> Const.t option diff --git a/infer/src/quandary/ClangTrace.ml b/infer/src/quandary/ClangTrace.ml index 24cc840f7..8fb01807e 100644 --- a/infer/src/quandary/ClangTrace.ml +++ b/infer/src/quandary/ClangTrace.ml @@ -286,9 +286,23 @@ module SinkKind = struct match Typ.Procname.to_string pname with | "creat" | "fopen" | "freopen" | "open" -> taint_nth 0 CreateFile actuals - | "curl_easy_setopt" -> - (* first two actuals are curl object + a constant *) - taint_after_nth 1 Network actuals + | "curl_easy_setopt" + -> ( + (* magic constant for setting request URL *) + let curlopt_url = 10002 in + (* first two actuals are curl object + integer code for data kind. *) + match List.nth actuals 1 with + | Some exp -> ( + match HilExp.eval exp with + | Some Const.Cint i -> + (* check if the data kind might be CURLOPT_URL *) + if Int.equal (IntLit.to_int i) curlopt_url then taint_after_nth 1 Network actuals + else None + | _ -> + (* can't statically resolve data kind; taint it just in case *) + taint_after_nth 1 Network actuals ) + | None -> + None ) | "execl" | "execlp" | "execle" | "execv" | "execve" | "execvp" | "system" -> taint_all ShellExec actuals | "openat" -> diff --git a/infer/tests/codetoanalyze/cpp/quandary/endpoints.cpp b/infer/tests/codetoanalyze/cpp/quandary/endpoints.cpp index 45f142949..0d5d996c8 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/endpoints.cpp +++ b/infer/tests/codetoanalyze/cpp/quandary/endpoints.cpp @@ -110,6 +110,22 @@ class Service1 : facebook::fb303::cpp2::FacebookServiceSvIf { curl_easy_setopt(nullptr, CURLOPT_URL, formal.s.c_str()); } + void endpoint_to_curl_url_exp_bad(request formal) { + curl_easy_setopt(nullptr, 10000 + 2, formal.s.c_str()); + } + + void endpoint_to_curl_url_unknown_exp_bad(request formal, int i) { + curl_easy_setopt(nullptr, i + 17, formal.s.c_str()); + } + + void endpoint_to_curl_other_const_ok(request formal) { + curl_easy_setopt(nullptr, 0, formal.s.c_str()); + } + + void endpoint_to_curl_other_exp_ok(request formal) { + curl_easy_setopt(nullptr, 1 + 2, formal.s.c_str()); + } + void FP_service1_endpoint_struct_int_field_ok(request formal) { system(std::to_string(formal.i).c_str()); } diff --git a/infer/tests/codetoanalyze/cpp/quandary/issues.exp b/infer/tests/codetoanalyze/cpp/quandary/issues.exp index d16794156..ea66d8516 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/issues.exp +++ b/infer/tests/codetoanalyze/cpp/quandary/issues.exp @@ -38,6 +38,8 @@ codetoanalyze/cpp/quandary/basics.cpp, basics::via_passthrough_bad1, 4, QUANDARY codetoanalyze/cpp/quandary/basics.cpp, basics::via_passthrough_bad2, 3, QUANDARY_TAINT_ERROR, [Return from basics::Obj_string_source,Call to basics::Obj_string_sink] codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_FP_service1_endpoint_struct_int_field_ok, 1, REMOTE_CODE_EXECUTION_RISK, [Return from endpoints::Service1_FP_service1_endpoint_struct_int_field_ok,Call to system] codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_endpoint_to_curl_url_bad, 1, UNTRUSTED_URL_RISK, [Return from endpoints::Service1_endpoint_to_curl_url_bad,Call to curl_easy_setopt] +codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_endpoint_to_curl_url_exp_bad, 1, UNTRUSTED_URL_RISK, [Return from endpoints::Service1_endpoint_to_curl_url_exp_bad,Call to curl_easy_setopt] +codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_endpoint_to_curl_url_unknown_exp_bad, 1, UNTRUSTED_URL_RISK, [Return from endpoints::Service1_endpoint_to_curl_url_unknown_exp_bad,Call to curl_easy_setopt] codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_fstream_open_file_bad, 1, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_fstream_open_file_bad,Call to std::basic_fstream>_basic_fstream] codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_fstream_open_file_bad, 3, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_fstream_open_file_bad,Call to std::basic_fstream>_open] codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_ifstream_open_file_bad, 1, UNTRUSTED_FILE_RISK, [Return from endpoints::Service1_ifstream_open_file_bad,Call to std::basic_ifstream>_basic_ifstream]