Skip to content

Commit e66ca99

Browse files
danogdanog
authored
Merge pull request #11628 from danog/fix_conditional_taints
Fix conditional taints
2 parents b09282f + df97201 commit e66ca99

8 files changed

Lines changed: 73 additions & 118 deletions

File tree

‎src/Psalm/Codebase.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,12 @@ public function getOrRegisterTaint(string $taint_type, ?CodeLocation $location =
366366
throw new RuntimeException($err);
367367
}
368368
if ($taint_type[0] === '(') {
369-
throw new AssertionError('Conditional taints cannot be registered directly');
369+
$err = "Conditional taints cannot be used in this context";
370+
if ($location !== null) {
371+
IssueBuffer::maybeAdd(new InvalidDocblock($err, $location));
372+
return null;
373+
}
374+
throw new RuntimeException($err);
370375
}
371376
$id = 1 << ($this->taint_count++);
372377
$this->custom_taints[$id] = $taint_type;

‎src/Psalm/Internal/Analyzer/CommentAnalyzer.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,9 @@ private static function decorateVarDocblockComment(
244244
if (isset($parsed_docblock->tags['psalm-taint-escape'])) {
245245
foreach ($parsed_docblock->tags['psalm-taint-escape'] as $param) {
246246
$param = trim($param);
247+
if ($param[0] === '(') {
248+
continue;
249+
}
247250
try {
248251
$var_comment->removed_taints |= $codebase->getOrRegisterTaint($param);
249252
} catch (RuntimeException $e) {

‎src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ public static function parse(
353353
} elseif ($param[0] === '(') {
354354
$line_parts = CommentAnalyzer::splitDocLine($param);
355355

356-
$info->removed_taints[] = CommentAnalyzer::sanitizeDocblockType($line_parts[0]);
356+
$info->conditionally_removed_taints[] = CommentAnalyzer::sanitizeDocblockType($line_parts[0]);
357357
} else {
358358
$info->removed_taints[] = explode(' ', $param)[0];
359359
}

‎src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php‎

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -368,28 +368,27 @@ public static function addDocblockInfo(
368368
}
369369

370370
foreach ($docblock_info->removed_taints as $removed_taint) {
371-
if ($removed_taint[0] === '(') {
372-
self::handleConditionallyRemovedTaint(
373-
$codebase,
374-
$stmt,
375-
$aliases,
376-
$removed_taint,
377-
$function_template_types,
378-
$class_template_types,
379-
$type_aliases,
380-
$storage,
381-
$classlike_storage,
382-
$cased_function_id,
383-
$file_storage,
384-
$file_scanner,
385-
);
386-
} else {
387-
$t = $codebase->getOrRegisterTaint($removed_taint, $location);
388-
if ($t !== null) {
389-
$storage->removed_taints |= $t;
390-
}
371+
$t = $codebase->getOrRegisterTaint($removed_taint, $location);
372+
if ($t !== null) {
373+
$storage->removed_taints |= $t;
391374
}
392375
}
376+
foreach ($docblock_info->conditionally_removed_taints as $removed_taint) {
377+
self::handleConditionallyRemovedTaint(
378+
$codebase,
379+
$stmt,
380+
$aliases,
381+
$removed_taint,
382+
$function_template_types,
383+
$class_template_types,
384+
$type_aliases,
385+
$storage,
386+
$classlike_storage,
387+
$cased_function_id,
388+
$file_storage,
389+
$file_scanner,
390+
);
391+
}
393392

394393
self::handleTaintFlow($docblock_info, $storage);
395394

‎src/Psalm/Internal/Scanner/FunctionDocblockComment.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,12 @@ final class FunctionDocblockComment
9999
public array $added_taints = [];
100100

101101
/**
102-
* @var array<string>
102+
* @var list<string>
103+
*/
104+
public array $conditionally_removed_taints = [];
105+
106+
/**
107+
* @var list<string>
103108
*/
104109
public array $removed_taints = [];
105110

‎src/Psalm/Type/TaintKind.php‎

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,36 @@
1212
*/
1313
final class TaintKind
1414
{
15+
public const INPUT_CALLABLE = (1 << 0);
16+
public const INPUT_UNSERIALIZE = (1 << 1);
17+
public const INPUT_INCLUDE = (1 << 2);
18+
public const INPUT_EVAL = (1 << 3);
19+
public const INPUT_LDAP = (1 << 4);
20+
public const INPUT_SQL = (1 << 5);
21+
public const INPUT_HTML = (1 << 6);
22+
public const INPUT_HAS_QUOTES = (1 << 7);
23+
public const INPUT_SHELL = (1 << 8);
24+
public const INPUT_SSRF = (1 << 9);
25+
public const INPUT_FILE = (1 << 10);
26+
public const INPUT_COOKIE = (1 << 11);
27+
public const INPUT_HEADER = (1 << 12);
28+
public const INPUT_XPATH = (1 << 13);
29+
public const INPUT_SLEEP = (1 << 14);
30+
public const INPUT_EXTRACT = (1 << 15);
31+
public const USER_SECRET = (1 << 16);
32+
public const SYSTEM_SECRET = (1 << 17);
33+
34+
public const ALL_INPUT = (1 << 16) - 1;
35+
36+
/** @internal */
37+
public const NUMERIC_ONLY = self::INPUT_SLEEP;
38+
/** @internal */
39+
public const BOOL_ONLY = self::INPUT_SLEEP;
40+
41+
/** @internal Keep this synced with the above */
42+
public const BUILTIN_TAINT_COUNT = 18;
43+
44+
1545
// Map of taint kind names to their bitmask values, used in taint annotations
1646
public const TAINT_NAMES = [
1747
'callable' => self::INPUT_CALLABLE,
@@ -33,36 +63,9 @@ final class TaintKind
3363
'user_secret' => self::USER_SECRET,
3464
'system_secret' => self::SYSTEM_SECRET,
3565

66+
'input_except_sleep' => self::ALL_INPUT & ~self::INPUT_SLEEP,
67+
3668
'input' => self::ALL_INPUT,
3769
'tainted' => self::ALL_INPUT,
3870
];
39-
40-
public const INPUT_CALLABLE = (1 << 0);
41-
public const INPUT_UNSERIALIZE = (1 << 2);
42-
public const INPUT_INCLUDE = (1 << 3);
43-
public const INPUT_EVAL = (1 << 4);
44-
public const INPUT_LDAP = (1 << 5);
45-
public const INPUT_SQL = (1 << 6);
46-
public const INPUT_HTML = (1 << 7);
47-
public const INPUT_HAS_QUOTES = (1 << 8);
48-
public const INPUT_SHELL = (1 << 9);
49-
public const INPUT_SSRF = (1 << 10);
50-
public const INPUT_FILE = (1 << 11);
51-
public const INPUT_COOKIE = (1 << 12);
52-
public const INPUT_HEADER = (1 << 13);
53-
public const INPUT_XPATH = (1 << 14);
54-
public const INPUT_SLEEP = (1 << 15);
55-
public const INPUT_EXTRACT = (1 << 16);
56-
public const USER_SECRET = (1 << 17);
57-
public const SYSTEM_SECRET = (1 << 18);
58-
59-
public const ALL_INPUT = (1 << 17) - 1;
60-
61-
/** @internal */
62-
public const NUMERIC_ONLY = self::INPUT_SLEEP;
63-
/** @internal */
64-
public const BOOL_ONLY = self::INPUT_SLEEP;
65-
66-
/** @internal Keep this synced with the above */
67-
public const BUILTIN_TAINT_COUNT = 19;
6871
}

‎stubs/CoreGenericFunctions.phpstub‎

Lines changed: 5 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -832,79 +832,19 @@ function array_product(array $array) {}
832832
* @psalm-pure
833833
*
834834
* 257 is FILTER_VALIDATE_INT
835-
* @psalm-taint-escape ($filter is 257 ? 'callable' : null)
836-
* @psalm-taint-escape ($filter is 257 ? 'unserialize' : null)
837-
* @psalm-taint-escape ($filter is 257 ? 'include' : null)
838-
* @psalm-taint-escape ($filter is 257 ? 'eval' : null)
839-
* @psalm-taint-escape ($filter is 257 ? 'ldap' : null)
840-
* @psalm-taint-escape ($filter is 257 ? 'sql' : null)
841-
* @psalm-taint-escape ($filter is 257 ? 'html' : null)
842-
* @psalm-taint-escape ($filter is 257 ? 'has_quotes' : null)
843-
* @psalm-taint-escape ($filter is 257 ? 'shell' : null)
844-
* @psalm-taint-escape ($filter is 257 ? 'ssrf' : null)
845-
* @psalm-taint-escape ($filter is 257 ? 'file' : null)
846-
* @psalm-taint-escape ($filter is 257 ? 'cookie' : null)
847-
* @psalm-taint-escape ($filter is 257 ? 'header' : null)
835+
* @psalm-taint-escape ($filter is 257 ? 'input_except_sleep' : null)
848836
*
849837
* 258 is FILTER_VALIDATE_BOOLEAN
850-
* @psalm-taint-escape ($filter is 258 ? 'callable' : null)
851-
* @psalm-taint-escape ($filter is 258 ? 'unserialize' : null)
852-
* @psalm-taint-escape ($filter is 258 ? 'include' : null)
853-
* @psalm-taint-escape ($filter is 258 ? 'eval' : null)
854-
* @psalm-taint-escape ($filter is 258 ? 'ldap' : null)
855-
* @psalm-taint-escape ($filter is 258 ? 'sql' : null)
856-
* @psalm-taint-escape ($filter is 258 ? 'html' : null)
857-
* @psalm-taint-escape ($filter is 258 ? 'has_quotes' : null)
858-
* @psalm-taint-escape ($filter is 258 ? 'shell' : null)
859-
* @psalm-taint-escape ($filter is 258 ? 'ssrf' : null)
860-
* @psalm-taint-escape ($filter is 258 ? 'file' : null)
861-
* @psalm-taint-escape ($filter is 258 ? 'cookie' : null)
862-
* @psalm-taint-escape ($filter is 258 ? 'header' : null)
838+
* @psalm-taint-escape ($filter is 258 ? 'input' : null)
863839
*
864840
* 259 is FILTER_VALIDATE_FLOAT
865-
* @psalm-taint-escape ($filter is 259 ? 'callable' : null)
866-
* @psalm-taint-escape ($filter is 259 ? 'unserialize' : null)
867-
* @psalm-taint-escape ($filter is 259 ? 'include' : null)
868-
* @psalm-taint-escape ($filter is 259 ? 'eval' : null)
869-
* @psalm-taint-escape ($filter is 259 ? 'ldap' : null)
870-
* @psalm-taint-escape ($filter is 259 ? 'sql' : null)
871-
* @psalm-taint-escape ($filter is 259 ? 'html' : null)
872-
* @psalm-taint-escape ($filter is 259 ? 'has_quotes' : null)
873-
* @psalm-taint-escape ($filter is 259 ? 'shell' : null)
874-
* @psalm-taint-escape ($filter is 259 ? 'ssrf' : null)
875-
* @psalm-taint-escape ($filter is 259 ? 'file' : null)
876-
* @psalm-taint-escape ($filter is 259 ? 'cookie' : null)
877-
* @psalm-taint-escape ($filter is 259 ? 'header' : null)
841+
* @psalm-taint-escape ($filter is 259 ? 'input_except_sleep' : null)
878842
*
879843
* 519 is FILTER_SANITIZE_NUMBER_INT
880-
* @psalm-taint-escape ($filter is 519 ? 'callable' : null)
881-
* @psalm-taint-escape ($filter is 519 ? 'unserialize' : null)
882-
* @psalm-taint-escape ($filter is 519 ? 'include' : null)
883-
* @psalm-taint-escape ($filter is 519 ? 'eval' : null)
884-
* @psalm-taint-escape ($filter is 519 ? 'ldap' : null)
885-
* @psalm-taint-escape ($filter is 519 ? 'sql' : null)
886-
* @psalm-taint-escape ($filter is 519 ? 'html' : null)
887-
* @psalm-taint-escape ($filter is 519 ? 'has_quotes' : null)
888-
* @psalm-taint-escape ($filter is 519 ? 'shell' : null)
889-
* @psalm-taint-escape ($filter is 519 ? 'ssrf' : null)
890-
* @psalm-taint-escape ($filter is 519 ? 'file' : null)
891-
* @psalm-taint-escape ($filter is 519 ? 'cookie' : null)
892-
* @psalm-taint-escape ($filter is 519 ? 'header' : null)
844+
* @psalm-taint-escape ($filter is 519 ? 'input_except_sleep' : null)
893845
*
894846
* 520 is FILTER_SANITIZE_NUMBER_FLOAT
895-
* @psalm-taint-escape ($filter is 520 ? 'callable' : null)
896-
* @psalm-taint-escape ($filter is 520 ? 'unserialize' : null)
897-
* @psalm-taint-escape ($filter is 520 ? 'include' : null)
898-
* @psalm-taint-escape ($filter is 520 ? 'eval' : null)
899-
* @psalm-taint-escape ($filter is 520 ? 'ldap' : null)
900-
* @psalm-taint-escape ($filter is 520 ? 'sql' : null)
901-
* @psalm-taint-escape ($filter is 520 ? 'html' : null)
902-
* @psalm-taint-escape ($filter is 520 ? 'has_quotes' : null)
903-
* @psalm-taint-escape ($filter is 520 ? 'shell' : null)
904-
* @psalm-taint-escape ($filter is 520 ? 'ssrf' : null)
905-
* @psalm-taint-escape ($filter is 520 ? 'file' : null)
906-
* @psalm-taint-escape ($filter is 520 ? 'cookie' : null)
907-
* @psalm-taint-escape ($filter is 520 ? 'header' : null)
847+
* @psalm-taint-escape ($filter is 520 ? 'input_except_sleep' : null)
908848
*
909849
* @psalm-flow ($value, $filter, $options) -> return
910850
*/

‎tests/TaintTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ function foo() : void {
260260
],
261261
'taintFilterVar' => [
262262
'code' => '<?php
263-
/** @psalm-taint-sink input $value */
263+
/** @psalm-taint-sink input_except_sleep $value */
264264
function taintSink(mixed $value): void {}
265265
266266
taintSink(filter_var($_GET["bad"], FILTER_VALIDATE_INT));

0 commit comments

Comments
 (0)