Skip to content

Commit

Permalink
Allow taints to be removed via annotation
Browse files Browse the repository at this point in the history
  • Loading branch information
@muglug
muglug committed Oct 15, 2019
1 parent 22a1244 commit b29227a
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 8 deletions.
2 changes: 2 additions & 0 deletions src/Psalm/DocComment.php
View file
Expand Up @@ -151,6 +151,7 @@ public static function parse($docblock, $line_number = null, $preserve_format =
'ignore-variable-method', 'ignore-variable-property', 'internal',
'taint-sink', 'taint-source', 'assert-untainted', 'scope-this',
'mutation-free', 'external-mutation-free', 'immutable', 'readonly',
'remove-taint',
],
true
)) {
Expand Down Expand Up @@ -274,6 +275,7 @@ public static function parsePreservingLength(\PhpParser\Comment\Doc $docblock)
'ignore-variable-method', 'ignore-variable-property', 'internal',
'taint-sink', 'taint-source', 'assert-untainted', 'scope-this',
'mutation-free', 'external-mutation-free', 'immutable', 'readonly',
'remove-taint',
],
true
)) {
Expand Down
10 changes: 8 additions & 2 deletions src/Psalm/Internal/Analyzer/CommentAnalyzer.php
View file
Expand Up @@ -181,6 +181,8 @@ public static function arrayToDocblocks(
$var_comment->internal = isset($parsed_docblock['specials']['internal']);
$var_comment->readonly = isset($parsed_docblock['specials']['readonly'])
|| isset($parsed_docblock['specials']['psalm-readonly']);
$var_comment->remove_taint = isset($parsed_docblock['specials']['psalm-remove-taint']);

if (isset($parsed_docblock['specials']['psalm-internal'])) {
$psalm_internal = reset($parsed_docblock['specials']['psalm-internal']);
if ($psalm_internal) {
Expand All @@ -203,13 +205,15 @@ public static function arrayToDocblocks(
&& (isset($parsed_docblock['specials']['deprecated'])
|| isset($parsed_docblock['specials']['internal'])
|| isset($parsed_docblock['specials']['readonly'])
|| isset($parsed_docblock['specials']['psalm-readonly']))
|| isset($parsed_docblock['specials']['psalm-readonly'])
|| isset($parsed_docblock['specials']['psalm-remove-taint']))
) {
$var_comment = new VarDocblockComment();
$var_comment->deprecated = isset($parsed_docblock['specials']['deprecated']);
$var_comment->internal = isset($parsed_docblock['specials']['internal']);
$var_comment->readonly = isset($parsed_docblock['specials']['readonly'])
|| isset($parsed_docblock['specials']['psalm-readonly']);
$var_comment->remove_taint = isset($parsed_docblock['specials']['psalm-remove-taint']);

$var_comments[] = $var_comment;
}
Expand Down Expand Up @@ -510,7 +514,9 @@ public static function extractFunctionDocblockInfo(PhpParser\Comment\Doc $commen
}
}


if (isset($parsed_docblock['specials']['psalm-remove-taint'])) {
$info->remove_taint = true;
}

if (isset($parsed_docblock['specials']['psalm-suppress'])) {
foreach ($parsed_docblock['specials']['psalm-suppress'] as $offset => $suppress_entry) {
Expand Down
12 changes: 12 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
View file
Expand Up @@ -77,6 +77,8 @@ public static function analyze(

$codebase = $statements_analyzer->getCodebase();

$remove_taint = false;

if ($doc_comment) {
$file_path = $statements_analyzer->getRootFilePath();

Expand Down Expand Up @@ -115,6 +117,10 @@ public static function analyze(
}

foreach ($var_comments as $var_comment) {
if ($var_comment->remove_taint) {
$remove_taint = true;
}

if (!$var_comment->type) {
continue;
}
Expand Down Expand Up @@ -770,6 +776,12 @@ public static function analyze(

return $context->vars_in_scope[$var_id];
}

if ($remove_taint && $context->vars_in_scope[$var_id]->sources) {
$context->vars_in_scope[$var_id]->sources = null;

$context->vars_in_scope[$var_id]->tainted = null;
}
}

if (!$was_in_assignment) {
Expand Down
12 changes: 6 additions & 6 deletions src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php
View file
Expand Up @@ -178,7 +178,7 @@ public static function analyze(
$stmt,
$cased_method_id,
$inferred_type,
$storage->location
$storage
);

if ($storage->return_type && !$storage->return_type->hasMixed()) {
Expand Down Expand Up @@ -425,15 +425,15 @@ private static function handleTaints(
PhpParser\Node\Stmt\Return_ $stmt,
string $cased_method_id,
Type\Union $inferred_type,
CodeLocation $function_location
\Psalm\Storage\FunctionLikeStorage $storage
) : void {
if (!$codebase->taint || !$stmt->expr) {
if (!$codebase->taint || !$stmt->expr || !$storage->location || $storage->remove_taint) {
return;
}

$method_sink = new Sink(
strtolower($cased_method_id),
$function_location
$storage->location
);

if ($previous_sink = $codebase->taint->hasPreviousSink($method_sink, $suffixes)) {
Expand Down Expand Up @@ -489,7 +489,7 @@ function (Source $inferred_source) use ($previous_sink) {

$new_source = new Source(
strtolower($cased_method_id . '-' . $suffix),
$function_location
$storage->location
);

$new_source->parents = [$previous_source ?: $type_source];
Expand All @@ -499,7 +499,7 @@ function (Source $inferred_source) use ($previous_sink) {
} else {
$new_source = new Source(
strtolower($cased_method_id),
$function_location
$storage->location
);

$new_source->parents = [$previous_source ?: $type_source];
Expand Down
5 changes: 5 additions & 0 deletions src/Psalm/Internal/Scanner/FunctionDocblockComment.php
View file
Expand Up @@ -154,4 +154,9 @@ class FunctionDocblockComment
* @var bool
*/
public $external_mutation_free = false;

/**
* @var bool
*/
public $remove_taint = false;
}
5 changes: 5 additions & 0 deletions src/Psalm/Internal/Scanner/VarDocblockComment.php
View file
Expand Up @@ -65,4 +65,9 @@ class VarDocblockComment
* @var bool
*/
public $readonly = false;

/**
* @var bool
*/
public $remove_taint = false;
}
4 changes: 4 additions & 0 deletions src/Psalm/Internal/Visitor/ReflectorVisitor.php
View file
Expand Up @@ -2025,6 +2025,10 @@ private function registerFunctionLike(PhpParser\Node\FunctionLike $stmt, $fake_m
}
}

if ($docblock_info->remove_taint) {
$storage->remove_taint = true;
}

if ($docblock_info->ignore_nullable_return && $storage->return_type) {
$storage->return_type->ignore_nullable_issues = true;
}
Expand Down
5 changes: 5 additions & 0 deletions src/Psalm/Storage/FunctionLikeStorage.php
View file
Expand Up @@ -181,6 +181,11 @@ class FunctionLikeStorage
*/
public $pure = false;

/**
* @var bool
*/
public $remove_taint = false;

public function __toString()
{
return $this->getSignature(false);
Expand Down
68 changes: 68 additions & 0 deletions tests/TaintTest.php
View file
Expand Up @@ -934,6 +934,74 @@ public function foo(O1 $o) : void {
$this->analyzeFile('somefile.php', new Context());
}

public function testTaintOnStrReplaceCallRemoved() : void
{
$this->project_analyzer->trackTaintedInputs();

$this->addFile(
'somefile.php',
'<?php
class U {
/**
* @psalm-pure
* @psalm-remove-taint
*/
public static function shorten(string $s) : string {
return str_replace("foo", "bar", $s);
}
}
class V {}
class O1 {
public string $s;
public function __construct() {
$this->s = (string) $_GET["FOO"];
}
}
class V1 extends V {
public function foo(O1 $o) : void {
echo U::shorten($o->s);
}
}'
);

$this->analyzeFile('somefile.php', new Context());
}

public function testTaintOnStrReplaceCallRemovedInline() : void
{
$this->project_analyzer->trackTaintedInputs();

$this->addFile(
'somefile.php',
'<?php
class V {}
class O1 {
public string $s;
public function __construct() {
$this->s = (string) $_GET["FOO"];
}
}
class V1 extends V {
public function foo(O1 $o) : void {
/**
* @psalm-remove-taint
*/
$a = str_replace("foo", "bar", $o->s);
echo $a;
}
}'
);

$this->analyzeFile('somefile.php', new Context());
}

public function testTaintOnPregReplaceCall() : void
{
$this->expectException(\Psalm\Exception\CodeException::class);
Expand Down

0 comments on commit b29227a

Please sign in to comment.