-
Notifications
You must be signed in to change notification settings - Fork 568
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
1. `taint_assume_safe_functions` to assume that function calls do not propagate taint from their arguments to their result. 2. `taint_assume_safe_indexes` to assume that a tainted index does not cause an access expression to be tainted. Both should help reducing FPs. Also, `taint_assume_safe_functions` is meant to replace the `not_conflicting` option in `pattern-sanitizers`, but is not enough by itself. This e.g. `sink(not_tainted(tainted('a')))` will still be flagged due to how sinks work right now (anything within the range of the sink is considered a sink, also `tainted('a')`). We may also need to generalize `pattern-propagators` so you can enumerate the functions that propagate taint. Closes PA-1541 test plan: make test # added two tests
- Loading branch information
Showing
17 changed files
with
126 additions
and
26 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
taint-mode: Added two new rule `options` that help minimizing false positives. | ||
|
||
One is `taint_assume_safe_functions`, which makes Semgrep assume that function | ||
calls do *NOT* propagate taint from their arguments to their output. Otherwise, | ||
Semgrep always assumes that functions may propagate taint. This is intended to | ||
replace _not conflicting_ sanitizers (added in v0.69.0) in the future, but this | ||
will not happen in this release. | ||
|
||
Another is `taint_assume_safe_indexes`, which makes Semgrep assume that an | ||
array-access expression is safe even if the index expression is tainted. Otherwise | ||
Semgrep assumes that e.g. `a[i]` is tainted if `i` is tainted, even if `a` is not. | ||
|
||
Note that both options are disabled by default, that is, Semgrep makes worst-case | ||
assumptions unless told otherwise. However, this may change in the future. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
<?php | ||
|
||
function not_tainted($data) { | ||
// do stuff | ||
return '2'; | ||
} | ||
|
||
//Problem here is that `tainted('a')` is in itself considered a sink because | ||
//currently taint-mode considers anyting that falls within a sink-range to be | ||
//a sink... and the sink range here is whatever it matches `sink(...)`! | ||
//todook:tainted | ||
sink(not_tainted(tainted('a'))); | ||
|
||
$ok = not_tainted(tainted('a')); | ||
//OK: | ||
sink($ok); | ||
|
||
//ruleid:tainted | ||
sink(tainted('b')); | ||
|
||
$bad = tainted('b'); | ||
//ruleid:tainted | ||
sink($bad); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
rules: | ||
- id: tainted | ||
languages: | ||
- php | ||
message: Match | ||
mode: taint | ||
options: | ||
taint_assume_safe_functions: true | ||
pattern-sinks: | ||
- pattern: sink(...) | ||
pattern-sources: | ||
- pattern: tainted(...) | ||
severity: WARNING | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
a = safe() | ||
i = tainted() | ||
x = a[i] | ||
#ok:tainted | ||
sink(x) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
rules: | ||
- id: tainted | ||
languages: | ||
- python | ||
message: Match | ||
mode: taint | ||
options: | ||
taint_assume_safe_indexes: true | ||
pattern-sinks: | ||
- pattern: sink(...) | ||
pattern-sources: | ||
- pattern: tainted(...) | ||
severity: WARNING | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.