Skip to content

Commit

Permalink
[taint] Add taint propagation event to value history
Browse files Browse the repository at this point in the history
Summary: Add taint propagation event to value history to take into account taint propagation through `PropagateTaintFrom` and fix several FNs.

Reviewed By: dulmarod

Differential Revision:
D50167594

Privacy Context Container: L1208441

fbshipit-source-id: 4efb821743eb23e2116761b35667636531e9195d
  • Loading branch information
geralt-encore authored and facebook-github-bot committed Oct 11, 2023
1 parent b48817a commit fb42a04
Show file tree
Hide file tree
Showing 16 changed files with 37 additions and 22 deletions.
7 changes: 6 additions & 1 deletion infer/src/pulse/PulseAttribute.ml
Original file line number Diff line number Diff line change
Expand Up @@ -513,9 +513,14 @@ module Attribute = struct
in
MustNotBeTainted (TaintSinkSet.map add_call_to_sink sinks)
| PropagateTaintFrom taints_in ->
let add_propagation_event_to_history hist =
let hist = add_call_to_history hist in
let propagation_event = ValueHistory.TaintPropagated (call_location, timestamp) in
ValueHistory.sequence propagation_event hist
in
PropagateTaintFrom
(List.map taints_in ~f:(fun {v; history} ->
{v= subst v; history= add_call_to_history history} ) )
{v= subst v; history= add_propagation_event_to_history history} ) )
| ReturnedFromUnknown values ->
ReturnedFromUnknown (List.map values ~f:subst)
| Tainted tainted ->
Expand Down
2 changes: 2 additions & 0 deletions infer/src/pulse/PulseTaintOperations.ml
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,8 @@ let check_source_against_sink_policy location ~source source_times intra_procedu
let check = function
| ValueHistory.TaintSource (taint_item, _, _) ->
List.exists taint_item.kinds ~f:(source_matches_sink_policy sink_kind sink_policy)
| ValueHistory.TaintPropagated _ ->
true
| _ ->
false
in
Expand Down
7 changes: 6 additions & 1 deletion infer/src/pulse/PulseValueHistory.ml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type event =
| Returned of Location.t * Timestamp.t
| StructFieldAddressCreated of Fieldname.t RevList.t * Location.t * Timestamp.t
| TaintSource of TaintItem.t * Location.t * Timestamp.t
| TaintPropagated of Location.t * Timestamp.t
| VariableAccessed of Pvar.t * Location.t * Timestamp.t
| VariableDeclared of Pvar.t * Location.t * Timestamp.t

Expand Down Expand Up @@ -75,6 +76,7 @@ let location_of_event = function
| Returned (location, _)
| StructFieldAddressCreated (_, location, _)
| TaintSource (_, location, _)
| TaintPropagated (location, _)
| VariableAccessed (_, location, _)
| VariableDeclared (_, location, _) ->
location
Expand All @@ -93,6 +95,7 @@ let timestamp_of_event = function
| Returned (_, timestamp)
| StructFieldAddressCreated (_, _, timestamp)
| TaintSource (_, _, timestamp)
| TaintPropagated (_, timestamp)
| VariableAccessed (_, _, timestamp)
| VariableDeclared (_, _, timestamp) ->
timestamp
Expand Down Expand Up @@ -252,6 +255,8 @@ let pp_event_no_location fmt event =
F.fprintf fmt "struct field address `%a` created" pp_fields field_names
| TaintSource (taint_source, _, _) ->
F.fprintf fmt "source of the taint here: %a" TaintItem.pp taint_source
| TaintPropagated _ ->
F.fprintf fmt "taint propagated"
| VariableAccessed (pvar, _, _) ->
F.fprintf fmt "%a accessed here" pp_pvar pvar
| VariableDeclared (pvar, _, _) ->
Expand Down Expand Up @@ -310,7 +315,7 @@ let is_taint_event = function
| VariableAccessed _
| VariableDeclared _ ->
false
| TaintSource _ ->
| TaintSource _ | TaintPropagated _ ->
true


Expand Down
1 change: 1 addition & 0 deletions infer/src/pulse/PulseValueHistory.mli
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type event =
| Returned of Location.t * Timestamp.t
| StructFieldAddressCreated of Fieldname.t RevList.t * Location.t * Timestamp.t
| TaintSource of TaintItem.t * Location.t * Timestamp.t
| TaintPropagated of Location.t * Timestamp.t
| VariableAccessed of Pvar.t * Location.t * Timestamp.t
| VariableDeclared of Pvar.t * Location.t * Timestamp.t

Expand Down
6 changes: 4 additions & 2 deletions infer/tests/codetoanalyze/cpp/pulse/issues.exp
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ codetoanalyze/cpp/pulse/taint/basics.cpp, basics::via_field_ok2, 2, PULSE_UNNECE
codetoanalyze/cpp/pulse/taint/basics.cpp, basics::via_passthrough_bad1, 4, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value returned from `basics::Obj::string_source` with kind `SimpleSource`,in call to function `std::basic_string<char,std::char_traits<char>,std::allocator<char>>::basic_string` with no summary,value passed as argument `#1` to `basics::Obj::string_sink` with kind `SimpleSink`], source: basics::Obj::string_source, sink: basics::Obj::string_sink, tainted expression: UNKNOWN
codetoanalyze/cpp/pulse/taint/basics.cpp, basics::via_passthrough_bad1, 4, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
codetoanalyze/cpp/pulse/taint/basics.cpp, basics::via_passthrough_bad2, 2, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
codetoanalyze/cpp/pulse/taint/basics.cpp, basics::via_passthrough_bad2, 3, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value returned from `basics::Obj::string_source` with kind `SimpleSource`,value passed as argument `#1` to `basics::Obj::string_sink` with kind `SimpleSink`], source: basics::Obj::string_source, sink: basics::Obj::string_sink, tainted expression: UNKNOWN
codetoanalyze/cpp/pulse/taint/basics.cpp, basics::via_passthrough_bad2, 3, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value returned from `basics::Obj::string_source` with kind `SimpleSource`,in call to function `std::basic_string<char,std::char_traits<char>,std::allocator<char>>::basic_string` with no summary,value passed as argument `#1` to `basics::Obj::string_sink` with kind `SimpleSink`], source: basics::Obj::string_source, sink: basics::Obj::string_sink, tainted expression: UNKNOWN
codetoanalyze/cpp/pulse/taint/basics.cpp, basics::via_passthrough_bad2, 3, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
codetoanalyze/cpp/pulse/taint/basics.cpp, basics::taint_arg_source_bad, 3, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value passed as argument `#0` to `basics::Obj::taint_arg_source` with kind `SimpleSource`,value passed as argument `#0` to `__infer_taint_sink` with kind `SimpleSink`], source: basics::Obj::taint_arg_source, sink: __infer_taint_sink, tainted expression: source
codetoanalyze/cpp/pulse/taint/basics.cpp, basics::via_sanitizer_ok1, 3, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
Expand Down Expand Up @@ -354,7 +354,9 @@ codetoanalyze/cpp/pulse/taint/strings.cpp, strings::format1<std::string_&>, 0, P
codetoanalyze/cpp/pulse/taint/strings.cpp, strings::format1<std::string_&>, 1, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [first instantiated at,copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
codetoanalyze/cpp/pulse/taint/strings.cpp, strings::format2<std::string_&>, 0, PULSE_CONST_REFABLE, no_bucket, ERROR, [Parameter fmt with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>`]
codetoanalyze/cpp/pulse/taint/strings.cpp, strings::format2<std::string_&>, 1, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [first instantiated at,copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
codetoanalyze/cpp/pulse/taint/strings.cpp, strings::format1_bad, 3, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value returned from `__infer_taint_source` with kind `SimpleSource`,value passed as argument `#0` to `__infer_taint_sink` with kind `SimpleSink`], source: __infer_taint_source, sink: __infer_taint_sink, tainted expression: UNKNOWN
codetoanalyze/cpp/pulse/taint/strings.cpp, strings::format1_bad, 3, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
codetoanalyze/cpp/pulse/taint/strings.cpp, strings::format2_bad, 3, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value returned from `__infer_taint_source` with kind `SimpleSource`,value passed as argument `#0` to `__infer_taint_sink` with kind `SimpleSink`], source: __infer_taint_source, sink: __infer_taint_sink, tainted expression: UNKNOWN
codetoanalyze/cpp/pulse/taint/strings.cpp, strings::format2_bad, 3, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
codetoanalyze/cpp/pulse/taint/strings.cpp, strings::format3_bad, 3, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value returned from `__infer_taint_source` with kind `SimpleSource`,in call to function `strings::format3<std::string_&>` with no summary,in call to function `strings::Formatter<std::basic_string<char,std::char_traits<char>,std::allocator<char>>&>::str` with no summary,in call to function `std::basic_string<char,std::char_traits<char>,std::allocator<char>>::basic_string` with no summary,value passed as argument `#0` to `__infer_taint_sink` with kind `SimpleSink`], source: __infer_taint_source, sink: __infer_taint_sink, tainted expression: UNKNOWN
codetoanalyze/cpp/pulse/taint/strings.cpp, strings::format3_bad, 3, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
Expand Down Expand Up @@ -383,7 +385,7 @@ codetoanalyze/cpp/pulse/taint/unknown_code.cpp, unknown_code::skip_indirect, 0,
codetoanalyze/cpp/pulse/taint/unknown_code.cpp, unknown_code::skip_indirect, 1, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
codetoanalyze/cpp/pulse/taint/unknown_code.cpp, unknown_code::skip_indirect, 2, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
codetoanalyze/cpp/pulse/taint/unknown_code.cpp, unknown_code::skip_indirect_bad, 2, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
codetoanalyze/cpp/pulse/taint/unknown_code.cpp, unknown_code::skip_indirect_bad, 3, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value returned from `__infer_taint_source` with kind `SimpleSource`,value passed as argument `#0` to `__infer_taint_sink` with kind `SimpleSink`], source: __infer_taint_source, sink: __infer_taint_sink, tainted expression: UNKNOWN
codetoanalyze/cpp/pulse/taint/unknown_code.cpp, unknown_code::skip_indirect_bad, 3, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value returned from `__infer_taint_source` with kind `SimpleSource`,in call to function `std::basic_string<char,std::char_traits<char>,std::allocator<char>>::basic_string` with no summary,value passed as argument `#0` to `__infer_taint_sink` with kind `SimpleSink`], source: __infer_taint_source, sink: __infer_taint_sink, tainted expression: UNKNOWN
codetoanalyze/cpp/pulse/taint/unknown_code.cpp, unknown_code::skip_indirect_bad, 3, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
codetoanalyze/cpp/pulse/taint/unknown_code.cpp, unknown_code::FN_via_skip_by_ref_bad, 3, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
codetoanalyze/cpp/pulse/taint/unknown_code.cpp, unknown_code::FN_via_skip_by_ref_bad, 4, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::basic_string<char,std::char_traits<char>,std::allocator<char>>&`)]
Expand Down
Loading

0 comments on commit fb42a04

Please sign in to comment.