From 164fa457e9589b4fbd319f9976f6009b8ecbd47f Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Wed, 22 Nov 2017 09:20:49 -0800 Subject: [PATCH] [quandary] treat any non-primitive endpoint formal as a source Reviewed By: mbouaziz Differential Revision: D6385271 fbshipit-source-id: 3360b04 --- infer/src/quandary/ClangTrace.ml | 14 ++++---------- .../tests/codetoanalyze/cpp/quandary/endpoints.cpp | 13 +++++++++++++ infer/tests/codetoanalyze/cpp/quandary/issues.exp | 2 ++ 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/infer/src/quandary/ClangTrace.ml b/infer/src/quandary/ClangTrace.ml index 2f69e0909..261a6b9bb 100644 --- a/infer/src/quandary/ClangTrace.ml +++ b/infer/src/quandary/ClangTrace.ml @@ -334,19 +334,13 @@ include Trace.Make (struct (* return true if code injection is possible because the source is a string/is not sanitized *) let is_injection_possible ?typ sanitizers = let is_escaped = List.mem sanitizers Sanitizer.Escape ~equal:Sanitizer.equal in - (* using this to match custom string wrappers such as folly::StringPiece *) - let is_stringy typ = - let lowercase_typ = String.lowercase (Typ.to_string (Typ.mk typ)) in - String.is_substring ~substring:"string" lowercase_typ - || String.is_substring ~substring:"char*" lowercase_typ - in not is_escaped && match typ with - | Some typ -> - is_stringy typ - | None -> - (* not sure if it's a string; assume injection possible *) + | Some (Typ.Tint _ | Tfloat _ | Tvoid) -> + false + | _ -> + (* possible a string/object/struct type; assume injection possible *) true diff --git a/infer/tests/codetoanalyze/cpp/quandary/endpoints.cpp b/infer/tests/codetoanalyze/cpp/quandary/endpoints.cpp index eeefa0456..fcc18289b 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/endpoints.cpp +++ b/infer/tests/codetoanalyze/cpp/quandary/endpoints.cpp @@ -26,6 +26,11 @@ class FacebookServiceSvAsyncIf {}; namespace endpoints { +struct request { + std::string s; + int i; +}; + class Service1 : facebook::fb303::cpp2::FacebookServiceSvIf { public: @@ -64,6 +69,14 @@ class Service1 : facebook::fb303::cpp2::FacebookServiceSvIf { system(__infer_string_sanitizer(formal).c_str()); } + void service1_endpoint_struct_string_field_bad(request formal) { + system(formal.s.c_str()); + } + + void FP_service1_endpoint_struct_int_field_ok(request formal) { + system(std::to_string(formal.i).c_str()); + } + private: void private_not_endpoint_ok(std::string formal) { system(formal.c_str()); } }; diff --git a/infer/tests/codetoanalyze/cpp/quandary/issues.exp b/infer/tests/codetoanalyze/cpp/quandary/issues.exp index 577efa06c..61cbce947 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/issues.exp +++ b/infer/tests/codetoanalyze/cpp/quandary/issues.exp @@ -36,8 +36,10 @@ codetoanalyze/cpp/quandary/basics.cpp, basics::via_field_bad1, 3, QUANDARY_TAINT codetoanalyze/cpp/quandary/basics.cpp, basics::via_field_bad2, 2, QUANDARY_TAINT_ERROR, [Return from basics::template_source_>,Call to basics::template_sink_>] codetoanalyze/cpp/quandary/basics.cpp, basics::via_passthrough_bad1, 4, QUANDARY_TAINT_ERROR, [Return from basics::Obj_string_source,Call to basics::Obj_string_sink] 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_sanitized_sql_bad, 2, USER_CONTROLLED_SQL_RISK, [Return from endpoints::Service1_sanitized_sql_bad,Call to __infer_sql_sink] codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_service1_endpoint_bad, 2, REMOTE_CODE_EXECUTION_RISK, [Return from endpoints::Service1_service1_endpoint_bad,Call to system] +codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_service1_endpoint_struct_string_field_bad, 1, REMOTE_CODE_EXECUTION_RISK, [Return from endpoints::Service1_service1_endpoint_struct_string_field_bad,Call to system] codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_unsanitized_sql_bad, 2, REMOTE_CODE_EXECUTION_RISK, [Return from endpoints::Service1_unsanitized_sql_bad,Call to __infer_sql_sink] codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_user_controlled_endpoint_to_shell_bad, 2, SHELL_INJECTION, [Return from endpoints::Service1_user_controlled_endpoint_to_shell_bad,Call to system] codetoanalyze/cpp/quandary/endpoints.cpp, endpoints::Service1_user_controlled_endpoint_to_sql_bad, 2, SQL_INJECTION, [Return from endpoints::Service1_user_controlled_endpoint_to_sql_bad,Call to __infer_sql_sink]