From 4e97d1e991a0a6d40538df92fa4c63e2e44329d0 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Thu, 11 May 2017 10:54:41 -0700 Subject: [PATCH] [quandary] add support for C++ parameter passing modes that differ from Java Summary: There are two pointer-related operations you can do in C++ but not Java that we need to support in taint analysis: (1) `*formal_ptr = ...` when `formal_ptr` is a formal that's a pointer type. Java doesn't have raw pointers, so we didn't need to handle this case. (2) Passing by reference, which Java also doesn't have (everything is pass-by-value). Reviewed By: mbouaziz Differential Revision: D5041246 fbshipit-source-id: 4e8f962 --- infer/src/quandary/TaintAnalysis.ml | 22 ++++-- .../codetoanalyze/cpp/quandary/basics.cpp | 3 +- .../codetoanalyze/cpp/quandary/issues.exp | 7 ++ .../codetoanalyze/cpp/quandary/pointers.cpp | 67 +++++++++++++++++++ 4 files changed, 90 insertions(+), 9 deletions(-) create mode 100644 infer/tests/codetoanalyze/cpp/quandary/pointers.cpp diff --git a/infer/src/quandary/TaintAnalysis.ml b/infer/src/quandary/TaintAnalysis.ml index 708897ea9..2e9c1d4f6 100644 --- a/infer/src/quandary/TaintAnalysis.ml +++ b/infer/src/quandary/TaintAnalysis.ml @@ -441,7 +441,11 @@ module Make (TaintSpecification : TaintSpec.S) = struct module Analyzer = AbstractInterpreter.Make (ProcCfg.Exceptional) (LowerHil.Make(TransferFunctions)) - let make_summary formal_map access_tree = + let make_summary { ProcData.pdesc; extras={ formal_map; } } access_tree = + let is_java = match Procdesc.get_proc_name pdesc with + | Java _ -> true + | _ -> false in + (* if a trace has footprint sources, attach them to the appropriate footprint var *) let access_tree' = TaintDomain.fold @@ -469,7 +473,13 @@ module Make (TaintSpecification : TaintSpec.S) = struct (* should only be used on nodes associated with a footprint base *) let is_empty_node (trace, tree) = - TraceDomain.Sinks.is_empty (TraceDomain.sinks trace) && + (* In C++, we can reassign the value pointed to by a pointer type formal, and we can assign + to a value type passed by reference. these mechanisms can be used to associate a source + directly with a formal. In Java this can't happen, so we only care if the formal flows to + a sink *) + (if is_java + then TraceDomain.Sinks.is_empty (TraceDomain.sinks trace) + else TraceDomain.is_empty trace) && match tree with | TaintDomain.Subtree subtree -> TaintDomain.AccessMap.is_empty subtree | TaintDomain.Star -> true in @@ -500,10 +510,8 @@ module Make (TaintSpecification : TaintSpec.S) = struct try TaintDomain.node_join (AccessPath.BaseMap.find base' acc) node with Not_found -> node in if is_empty_node joined_node - then - acc - else - AccessPath.BaseMap.add base' joined_node acc + then acc + else AccessPath.BaseMap.add base' joined_node acc | None -> (* base is a local var *) acc) @@ -542,7 +550,7 @@ module Make (TaintSpecification : TaintSpec.S) = struct let initial = make_initial proc_data.pdesc in match Analyzer.compute_post proc_data ~initial with | Some (access_tree, _) -> - Some (make_summary proc_data.extras.formal_map access_tree) + Some (make_summary proc_data access_tree) | None -> if Procdesc.Node.get_succs (Procdesc.get_start_node proc_data.pdesc) <> [] then failwith "Couldn't compute post" diff --git a/infer/tests/codetoanalyze/cpp/quandary/basics.cpp b/infer/tests/codetoanalyze/cpp/quandary/basics.cpp index 3069ffb1e..cb73904e0 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/basics.cpp +++ b/infer/tests/codetoanalyze/cpp/quandary/basics.cpp @@ -125,8 +125,7 @@ void via_passthrough_bad1(Obj* obj) { obj->string_sink(*laundered_source); } -// the summary for id2 doesn't assign to the return value -void FN_via_passthrough_bad2(Obj* obj) { +void via_passthrough_bad2(Obj* obj) { std::string source = obj->string_source(0); std::string laundered_source = id2(source); obj->string_sink(laundered_source); diff --git a/infer/tests/codetoanalyze/cpp/quandary/issues.exp b/infer/tests/codetoanalyze/cpp/quandary/issues.exp index c70d7065c..dd6e5acfa 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/issues.exp +++ b/infer/tests/codetoanalyze/cpp/quandary/issues.exp @@ -17,6 +17,7 @@ codetoanalyze/cpp/quandary/basics.cpp, basics::template_source_bad, 2, QUANDARY_ codetoanalyze/cpp/quandary/basics.cpp, basics::via_field_bad1, 3, QUANDARY_TAINT_ERROR, [return from basics::template_source_>,call to basics::template_sink_>] 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,flow through basics::id1_>,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,flow through basics::id2_>,call to basics::Obj_string_sink] codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 6, QUANDARY_TAINT_ERROR, [return from getenv,call to execl] codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 8, QUANDARY_TAINT_ERROR, [return from getenv,call to execl] codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 11, QUANDARY_TAINT_ERROR, [return from getenv,call to execl] @@ -32,3 +33,9 @@ codetoanalyze/cpp/quandary/files.cpp, files::read_file_call_exec_bad1, 5, QUANDA codetoanalyze/cpp/quandary/files.cpp, files::read_file_call_exec_bad2, 5, QUANDARY_TAINT_ERROR, [return from std::basic_istream>_readsome,call to execle] codetoanalyze/cpp/quandary/files.cpp, files::read_file_call_exec_bad3, 5, QUANDARY_TAINT_ERROR, [return from std::basic_istream>_getline,call to execle] codetoanalyze/cpp/quandary/files.cpp, files::read_file_call_exec_bad5, 4, QUANDARY_TAINT_ERROR, [return from std::basic_istream>_getline,call to execle] +codetoanalyze/cpp/quandary/pointers.cpp, pointers::FP_reuse_pointer_as_local_ok, 2, QUANDARY_TAINT_ERROR, [return from __infer_taint_source,return from pointers::reuse_pointer_as_local,call to __infer_taint_sink] +codetoanalyze/cpp/quandary/pointers.cpp, pointers::assign_pointer_pass_to_sink_bad1, 2, QUANDARY_TAINT_ERROR, [return from __infer_taint_source,return from pointers::assign_pointer_to_source,call to __infer_taint_sink] +codetoanalyze/cpp/quandary/pointers.cpp, pointers::assign_pointer_pass_to_sink_bad2, 3, QUANDARY_TAINT_ERROR, [return from __infer_taint_source,return from pointers::assign_pointer_to_source,call to __infer_taint_sink] +codetoanalyze/cpp/quandary/pointers.cpp, pointers::assign_source_by_reference_bad1, 3, QUANDARY_TAINT_ERROR, [return from __infer_taint_source,return from pointers::assign_source_by_reference,call to __infer_taint_sink] +codetoanalyze/cpp/quandary/pointers.cpp, pointers::assign_source_by_reference_bad2, 2, QUANDARY_TAINT_ERROR, [return from __infer_taint_source,return from pointers::assign_source_by_reference,call to __infer_taint_sink] +codetoanalyze/cpp/quandary/pointers.cpp, pointers::assign_source_by_reference_bad3, 3, QUANDARY_TAINT_ERROR, [return from __infer_taint_source,return from pointers::assign_source_by_reference,return from pointers::call_assign_source_by_reference,call to __infer_taint_sink] diff --git a/infer/tests/codetoanalyze/cpp/quandary/pointers.cpp b/infer/tests/codetoanalyze/cpp/quandary/pointers.cpp new file mode 100644 index 000000000..098e6692f --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/quandary/pointers.cpp @@ -0,0 +1,67 @@ +/* + * 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 + +extern std::string* __infer_taint_source(); +extern void __infer_taint_sink(std::string); + +namespace pointers { + +void assign_pointer_to_source(std::string* pointer) { + *pointer = *__infer_taint_source(); +} + +void assign_pointer_pass_to_sink_bad1(std::string* pointer) { + assign_pointer_to_source(pointer); + __infer_taint_sink(*pointer); +} + +void assign_pointer_pass_to_sink_bad2() { + std::string* pointer = new std::string(); + assign_pointer_to_source(pointer); + __infer_taint_sink(*pointer); +} + +void assign_source_by_reference(std::string& reference) { + reference = *__infer_taint_source(); +} + +void assign_source_by_reference_bad1() { + std::string local; + assign_source_by_reference(local); + __infer_taint_sink(local); +} + +void assign_source_by_reference_bad2(std::string formal) { + assign_source_by_reference(formal); + __infer_taint_sink(formal); +} + +void call_assign_source_by_reference(std::string& formal) { + assign_source_by_reference(formal); +} + +void assign_source_by_reference_bad3() { + std::string local; + call_assign_source_by_reference(local); + __infer_taint_sink(local); +} + +void reuse_pointer_as_local(std::string* pointer) { + pointer = __infer_taint_source(); +} + +// need to understand that assigning a reference doesn't change the value in the +// caller +void FP_reuse_pointer_as_local_ok(std::string* pointer) { + reuse_pointer_as_local(pointer); + __infer_taint_sink(*pointer); +} +}