Skip to content

Commit

Permalink
Use TaintKind/TaintKindGroup constants instead of string values
Browse files Browse the repository at this point in the history
  • Loading branch information
ohader committed Feb 24, 2024
1 parent eeffee7 commit 6f155c9
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
use Psalm\Type\Atomic\TNonEmptyArray;
use Psalm\Type\Atomic\TNull;
use Psalm\Type\Atomic\TString;
use Psalm\Type\TaintKind;
use Psalm\Type\Union;
use UnexpectedValueException;

Expand Down Expand Up @@ -646,9 +647,9 @@ private static function taintReturnType(
$pattern = substr($pattern, 2, -1);

if (self::simpleExclusion($pattern, $first_arg_value[0])) {
$removed_taints[] = 'html';
$removed_taints[] = 'has_quotes';
$removed_taints[] = 'sql';
$removed_taints[] = TaintKind::INPUT_HTML;
$removed_taints[] = TaintKind::INPUT_HAS_QUOTES;
$removed_taints[] = TaintKind::INPUT_SQL;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Psalm\Internal\Scanner\ParsedDocblock;
use Psalm\Issue\InvalidDocblock;
use Psalm\IssueBuffer;
use Psalm\Type\TaintKindGroup;

use function array_keys;
use function array_shift;
Expand Down Expand Up @@ -264,10 +265,11 @@ public static function parse(
$taint_type = substr($taint_type, 5);

if ($taint_type === 'tainted') {
$taint_type = 'input';
$taint_type = TaintKindGroup::GROUP_INPUT;
}

if ($taint_type === 'misc') {
// @todo `text` is semantically not defined in `TaintKind`, maybe drop it
$taint_type = 'text';
}

Expand Down Expand Up @@ -305,10 +307,11 @@ public static function parse(

if ($param_parts[0]) {
if ($param_parts[0] === 'tainted') {
$param_parts[0] = 'input';
$param_parts[0] = TaintKindGroup::GROUP_INPUT;
}

if ($param_parts[0] === 'misc') {
// @todo `text` is semantically not defined in `TaintKind`, maybe drop it
$param_parts[0] = 'text';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@

use function array_filter;
use function array_merge;
use function array_search;
use function array_splice;
use function array_unique;
use function array_values;
use function count;
use function explode;
Expand Down Expand Up @@ -352,16 +355,21 @@ public static function addDocblockInfo(
}
}

foreach ($docblock_info->taint_source_types as $taint_source_type) {
if ($taint_source_type === 'input') {
$storage->taint_source_types = array_merge(
$storage->taint_source_types,
TaintKindGroup::ALL_INPUT,
);
} else {
$storage->taint_source_types[] = $taint_source_type;
}
$docblock_info->taint_source_types = array_unique($docblock_info->taint_source_types);
// expand 'input' group to all items, e.g. `['other', 'input']` -> `['other', 'html', 'sql', 'shell', ...]`
$inputIndex = array_search(TaintKindGroup::GROUP_INPUT, $docblock_info->taint_source_types, true);
if ($inputIndex !== false) {
array_splice(
$docblock_info->taint_source_types,
$inputIndex,

Check failure on line 364 in src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php

View workflow job for this annotation

GitHub Actions / build

MixedArgumentTypeCoercion

src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php:364:17: MixedArgumentTypeCoercion: Argument 2 of array_splice expects int, but parent type array-key provided (see https://psalm.dev/194)
1,
TaintKindGroup::ALL_INPUT,
);
}
// merge taints from doc block to storage, enforce uniqueness and having consecutive index keys
$storage->taint_source_types = array_merge($storage->taint_source_types, $docblock_info->taint_source_types);
$storage->taint_source_types = array_unique($storage->taint_source_types);
$storage->taint_source_types = array_values($storage->taint_source_types);

$storage->added_taints = $docblock_info->added_taints;

Expand Down
21 changes: 11 additions & 10 deletions src/Psalm/Internal/Provider/AddRemoveTaints/HtmlFunctionTainter.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Psalm\Plugin\EventHandler\AddTaintsInterface;
use Psalm\Plugin\EventHandler\Event\AddRemoveTaintsEvent;
use Psalm\Plugin\EventHandler\RemoveTaintsInterface;
use Psalm\Type\TaintKind;

use function count;
use function strtolower;
Expand Down Expand Up @@ -47,24 +48,24 @@ public static function addTaints(AddRemoveTaintsEvent $event): array

if ($second_arg === null) {
if ($statements_analyzer->getCodebase()->analysis_php_version_id >= 8_01_00) {
return ['html', 'has_quotes'];
return [TaintKind::INPUT_HTML, TaintKind::INPUT_HAS_QUOTES];
}
return ['html'];
return [TaintKind::INPUT_HTML];
}

$second_arg_value = $statements_analyzer->node_data->getType($second_arg);

if (!$second_arg_value || !$second_arg_value->isSingleIntLiteral()) {
return ['html'];
return [TaintKind::INPUT_HTML];
}

$second_arg_value = $second_arg_value->getSingleIntLiteral()->value;

if (($second_arg_value & ENT_QUOTES) === ENT_QUOTES) {
return ['html', 'has_quotes'];
return [TaintKind::INPUT_HTML, TaintKind::INPUT_HAS_QUOTES];
}

return ['html'];
return [TaintKind::INPUT_HTML];
}

return [];
Expand Down Expand Up @@ -99,24 +100,24 @@ public static function removeTaints(AddRemoveTaintsEvent $event): array

if ($second_arg === null) {
if ($statements_analyzer->getCodebase()->analysis_php_version_id >= 8_01_00) {
return ['html', 'has_quotes'];
return [TaintKind::INPUT_HTML, TaintKind::INPUT_HAS_QUOTES];
}
return ['html'];
return [TaintKind::INPUT_HTML];
}

$second_arg_value = $statements_analyzer->node_data->getType($second_arg);

if (!$second_arg_value || !$second_arg_value->isSingleIntLiteral()) {
return ['html'];
return [TaintKind::INPUT_HTML];
}

$second_arg_value = $second_arg_value->getSingleIntLiteral()->value;

if (($second_arg_value & ENT_QUOTES) === ENT_QUOTES) {
return ['html', 'has_quotes'];
return [TaintKind::INPUT_HTML, TaintKind::INPUT_HAS_QUOTES];
}

return ['html'];
return [TaintKind::INPUT_HTML];
}

return [];
Expand Down
2 changes: 2 additions & 0 deletions src/Psalm/Type/TaintKindGroup.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
*/
final class TaintKindGroup
{
public const GROUP_INPUT = 'input';

public const ALL_INPUT = [
TaintKind::INPUT_HTML,
TaintKind::INPUT_HAS_QUOTES,
Expand Down

0 comments on commit 6f155c9

Please sign in to comment.