Skip to content

Commit

Permalink
[taint] Follow field accesses based on a config option
Browse files Browse the repository at this point in the history
Summary: While it does make sense to follow field access for taint propagation in some cases in other  cases it may result in FPs which are very hard to follow so this diff makes this behaviour configurable instead of unconditionally enable it based on a language.

Reviewed By: skcho

Differential Revision:
D62099585

Privacy Context Container: L1208441

fbshipit-source-id: 1b801ec5962b4f41cce18380b92207e0b67ac6fc
  • Loading branch information
geralt-encore authored and facebook-github-bot committed Sep 4, 2024
1 parent a9f9cd8 commit 19550b5
Show file tree
Hide file tree
Showing 13 changed files with 144 additions and 1 deletion.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ DIRECT_TESTS += \
BUILD_SYSTEMS_TESTS += \
differential_hack \
pulse_messages_hack \
pulse_taint_hack_no_follow_field_accesses \

endif

Expand Down
5 changes: 5 additions & 0 deletions infer/man/man1/infer-analyze.txt
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,11 @@ PULSE CHECKER OPTIONS
originate at the source will be reported. If a sink has such a
kind, only sensitive data flows to the sink will be reported.

--no-pulse-taint-follow-field-accesses
Deactivates: Specify if taint analysis should follow field
accesses when propagating taints. (Conversely:
--pulse-taint-follow-field-accesses)

--pulse-taint-opaque-files +path
Specify files that should be treated as opaque for taint analysis
to make sure that procedure's belonging to these files are always
Expand Down
6 changes: 6 additions & 0 deletions infer/man/man1/infer-full.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1644,6 +1644,12 @@ OPTIONS
kind, only sensitive data flows to the sink will be reported.
See also infer-analyze(1).

--no-pulse-taint-follow-field-accesses
Deactivates: Specify if taint analysis should follow field
accesses when propagating taints. (Conversely:
--pulse-taint-follow-field-accesses)
See also infer-analyze(1).

--pulse-taint-opaque-files +path
Specify files that should be treated as opaque for taint analysis
to make sure that procedure's belonging to these files are always
Expand Down
6 changes: 6 additions & 0 deletions infer/man/man1/infer.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1644,6 +1644,12 @@ OPTIONS
kind, only sensitive data flows to the sink will be reported.
See also infer-analyze(1).

--no-pulse-taint-follow-field-accesses
Deactivates: Specify if taint analysis should follow field
accesses when propagating taints. (Conversely:
--pulse-taint-follow-field-accesses)
See also infer-analyze(1).

--pulse-taint-opaque-files +path
Specify files that should be treated as opaque for taint analysis
to make sure that procedure's belonging to these files are always
Expand Down
8 changes: 8 additions & 0 deletions infer/src/base/Config.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2721,6 +2721,12 @@ and pulse_taint_data_flow_kinds =
such a kind, only sensitive data flows to the sink will be reported."


and pulse_taint_follow_field_accesses =
CLOpt.mk_bool ~long:"pulse-taint-follow-field-accesses" ~default:true
~in_help:InferCommand.[(Analyze, manual_pulse)]
"Specify if taint analysis should follow field accesses when propagating taints."


and pulse_taint_opaque_files =
CLOpt.mk_path_list ~long:"pulse-taint-opaque-files"
~in_help:InferCommand.[(Analyze, manual_pulse)]
Expand Down Expand Up @@ -4547,6 +4553,8 @@ and pulse_taint_config =

and pulse_taint_opaque_files = RevList.to_list !pulse_taint_opaque_files

and pulse_taint_follow_field_accesses = !pulse_taint_follow_field_accesses

and pulse_taint_short_traces = !pulse_taint_short_traces

and pulse_taint_skip_sources = !pulse_taint_skip_sources
Expand Down
2 changes: 2 additions & 0 deletions infer/src/base/Config.mli
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,8 @@ type pulse_taint_config =

val pulse_taint_config : pulse_taint_config

val pulse_taint_follow_field_accesses : bool

val pulse_taint_opaque_files : string list

val pulse_taint_short_traces : bool
Expand Down
3 changes: 2 additions & 1 deletion infer/src/pulse/PulseTaintOperations.ml
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,8 @@ let fold_taint_dependencies addr_hist0 astate ~init ~f =
| ArrayAccess _ ->
true
| FieldAccess _ ->
Language.curr_language_is Hack || Language.curr_language_is Python
Config.pulse_taint_follow_field_accesses
&& (Language.curr_language_is Hack || Language.curr_language_is Python)
| Dereference ->
false
in
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"scheduler": "callgraph",
"pulse-specialization-partial": true,
"pulse-taint-short-traces": true,
"pulse-taint-follow-field-accesses": false,
"pulse-taint-config": [
"taint_config.json"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Copyright (c) Facebook, Inc. and its affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

TESTS_DIR = ../..

INFER_OPTIONS = \
--pulse-only --debug-exceptions --no-pulse-force-continue

INFERPRINT_OPTIONS = --issues-tests

HH_SOURCES = $(sort $(wildcard hh/*.hack))
SOURCES = $(sort $(wildcard *.hack))

include $(TESTS_DIR)/hack.make
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright (c) Facebook, Inc. and its affiliates.
//
// This source code is licensed under the MIT license found in the
// LICENSE file in the root directory of this source tree.

namespace Shapes;

function unknown(mixed $sc): string {
return "42";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
shapes.hack, Shapes::C1.passViaShapeGetBad, 2, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value passed as argument `#0` to `Shapes::C1.passViaShapeGetBad` with kind `Shapes`,flows to this sink: value passed as argument `#0` to `Shapes::ShapeLogger$static.taintSink` with kind `Shapes`], source: Shapes::C1.passViaShapeGetBad, sink: Shapes::ShapeLogger$static.taintSink, tainted expression: $s->debug_data
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright (c) Facebook, Inc. and its affiliates.
//
// This source code is licensed under the MIT license found in the
// LICENSE file in the root directory of this source tree.

namespace Shapes;

class SensitiveClass {
public string $sensitiveField = "42";

public function getId(): string {
return $this->sensitiveField;
}
}

class ShapeLogger {
const type TSchemaShape = shape(
'msg' => string,
?'debug_data' => ?string,
);

public static function logData(this::TSchemaShape $data): void {
self::taintSink($data);
}

public static function taintSink(mixed $data): void {}
}

class C1 {
public function passViaShapeOk(SensitiveClass $sc): void {
ShapeLogger::logData(
shape('msg' => 'Oh-oh', 'debug_data' => $sc->sensitiveField),
);
}

public function passViaShapeGetBad(SensitiveClass $sc): void {
$s = shape('msg' => 'Oh-oh', 'debug_data' => $sc->sensitiveField);
ShapeLogger::taintSink($s['debug_data']);
}

public function passUnrelatedViaShapeGetOk(SensitiveClass $sc): void {
$s = shape('msg' => 'Oh-oh', 'debug_data' => $sc->sensitiveField);
ShapeLogger::taintSink($s['msg']);
}

public function passViaUnknownOk(SensitiveClass $sc): void {
$data = unknown($sc);
ShapeLogger::logData(shape('msg' => 'Oh-oh', 'debug_data' => $data));
}

public function passViaShapeAndUnknownOk(SensitiveClass $sc): void {
$data = unknown(shape("sc" => $sc));
ShapeLogger::logData(shape('msg' => 'Oh-oh', 'debug_data' => $data));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"pulse-taint-sources": [
{
"procedure_regex": ".*",
"taint_target": [
"ArgumentsMatchingTypes",
[
"SensitiveClass"
]
],
"kinds": ["Shapes"]
}
],
"pulse-taint-sinks": [
{ "class_names": ["Shapes::ShapeLogger"], "method_names": ["taintSink"], "kinds": ["Shapes"] }
],
"pulse-taint-policies": [
{
"short_description": "Taint flow",
"taint_flows": [{"source_kinds": ["Shapes"], "sink_kinds": ["Shapes"]}]
}
]
}

0 comments on commit 19550b5

Please sign in to comment.