diff --git a/config.xsd b/config.xsd index 1b5d3cceb..52be98941 100644 --- a/config.xsd +++ b/config.xsd @@ -309,6 +309,7 @@ + diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md index e4ddf15a0..2dfd3e1bd 100644 --- a/docs/running_psalm/issues.md +++ b/docs/running_psalm/issues.md @@ -2044,6 +2044,10 @@ Emitted when using a reserved word as a class name function foo(resource $res) : void {} ``` +### TaintedInput + +Emitted when tainted input detection is turned on + ### TraitMethodSignatureMismatch Emitted when a method's signature or return type differs from corresponding trait-defined method diff --git a/src/Psalm/CodeLocation.php b/src/Psalm/CodeLocation.php index bdf5f1745..0644fc8e1 100644 --- a/src/Psalm/CodeLocation.php +++ b/src/Psalm/CodeLocation.php @@ -404,4 +404,9 @@ class CodeLocation { return (string) $this->file_start; } + + public function getShortSummary() : string + { + return $this->file_name . ':' . $this->getLineNumber() . ':' . $this->getColumn(); + } } diff --git a/src/Psalm/Codebase.php b/src/Psalm/Codebase.php index 7808ce313..afc52a2c6 100644 --- a/src/Psalm/Codebase.php +++ b/src/Psalm/Codebase.php @@ -170,6 +170,11 @@ class Codebase */ public $populator; + /** + * @var ?Internal\Codebase\Taint + */ + public $taint = null; + /** * @var bool */ @@ -267,6 +272,8 @@ class Codebase */ public $php_minor_version = PHP_MINOR_VERSION; + + public function __construct( Config $config, Providers $providers, diff --git a/src/Psalm/DocComment.php b/src/Psalm/DocComment.php index 63f82c396..9dfb7bb44 100644 --- a/src/Psalm/DocComment.php +++ b/src/Psalm/DocComment.php @@ -150,6 +150,7 @@ class DocComment 'override-method-visibility', 'seal-properties', 'seal-methods', 'generator-return', 'ignore-falsable-return', 'variadic', 'pure', 'ignore-variable-method', 'ignore-variable-property', 'internal', + 'taint-sink', 'taint-source', 'assert-untainted', ], true )) { @@ -271,6 +272,7 @@ class DocComment 'override-method-visibility', 'seal-properties', 'seal-methods', 'generator-return', 'ignore-falsable-return', 'variadic', 'pure', 'ignore-variable-method', 'ignore-variable-property', 'internal', + 'taint-sink', 'taint-source', 'assert-untainted', ], true )) { diff --git a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php index 7b6aed845..01d62bcb3 100644 --- a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php @@ -429,6 +429,24 @@ class CommentAnalyzer } } + if (isset($parsed_docblock['specials']['psalm-taint-sink'])) { + /** @var string $param */ + foreach ($parsed_docblock['specials']['psalm-taint-sink'] as $param) { + $param = trim($param); + + $info->taint_sink_params[] = ['name' => $param]; + } + } + + if (isset($parsed_docblock['specials']['psalm-assert-untainted'])) { + /** @var string $param */ + foreach ($parsed_docblock['specials']['psalm-assert-untainted'] as $param) { + $param = trim($param); + + $info->assert_untainted_params[] = ['name' => $param]; + } + } + if (isset($parsed_docblock['specials']['global'])) { foreach ($parsed_docblock['specials']['global'] as $offset => $global) { $line_parts = self::splitDocLine($global); diff --git a/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php index cb4d98038..73f0e8298 100644 --- a/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php @@ -39,6 +39,7 @@ use function strtolower; use function substr; use function count; use function in_array; +use Psalm\Internal\Taint\TypeSource; /** * @internal diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php index 200a81af2..422ba7bcd 100644 --- a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php @@ -42,6 +42,7 @@ use function array_search; use function array_keys; use function end; use function array_diff; +use Psalm\Internal\Taint\TypeSource; /** * @internal @@ -267,7 +268,7 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer implements Statements $context->calling_method_id = strtolower($method_id); } elseif ($this->function instanceof Function_) { - $cased_method_id = $this->function->name; + $cased_method_id = $this->function->name->name; } else { // Closure if ($storage->return_type) { $closure_return_type = ExpressionAnalyzer::fleshOutType( @@ -591,6 +592,15 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer implements Statements ]); } + if ($cased_method_id && $codebase->taint) { + $type_source = TypeSource::getForMethodArgument($cased_method_id, $offset, $function_param->location); + $var_type->sources = [$type_source]; + + if ($codebase->taint->hasExistingSource($type_source)) { + $var_type->tainted = 1; + } + } + $context->vars_in_scope['$' . $function_param->name] = $var_type; $context->vars_possibly_in_scope['$' . $function_param->name] = true; diff --git a/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php b/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php index 9fbf99976..4caf9d239 100644 --- a/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php @@ -82,6 +82,7 @@ use function file_get_contents; use function substr_count; use function array_map; use function end; +use Psalm\Internal\Codebase\Taint; /** * @internal @@ -549,6 +550,14 @@ class ProjectAnalyzer ); } + /** + * @return void + */ + public function trackTaintedInputs() + { + $this->codebase->taint = new Taint(); + } + public function interpretRefactors() : void { if (!$this->codebase->alter_code) { diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php index 84c148980..75462a7c2 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php @@ -41,6 +41,7 @@ use function count; use function in_array; use function strtolower; use function explode; +use Psalm\Internal\Taint\TypeSource; /** * @internal @@ -455,6 +456,45 @@ class PropertyAssignmentAnalyzer } } + if ($codebase->taint) { + $method_source = new TypeSource( + $property_id, + new CodeLocation($statements_analyzer->getSource(), $stmt) + ); + + if ($codebase->taint->hasPreviousSink($method_source)) { + if ($assignment_value_type->sources) { + $codebase->taint->addSinks( + $statements_analyzer, + $assignment_value_type->sources, + new CodeLocation($statements_analyzer->getSource(), $stmt), + $method_source + ); + } + } + + if ($assignment_value_type->sources) { + foreach ($assignment_value_type->sources as $type_source) { + if ($codebase->taint->hasPreviousSource($type_source) + || $assignment_value_type->tainted + ) { + $codebase->taint->addSources( + $statements_analyzer, + [$method_source], + new CodeLocation($statements_analyzer->getSource(), $stmt), + $type_source + ); + } + } + } elseif ($assignment_value_type->tainted) { + throw new \UnexpectedValueException( + 'sources should exist for tainted var in ' + . $statements_analyzer->getFileName() . ':' + . $stmt->getLine() + ); + } + } + if (!$codebase->properties->propertyExists( $property_id, false, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php index 5c7503345..7f6c0f4b8 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php @@ -329,6 +329,29 @@ class BinaryOpAnalyzer if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->right, $context) === false) { return false; } + + if ($codebase->taint) { + $sources = []; + $either_tainted = 0; + + if (isset($stmt->left->inferredType)) { + $sources = $stmt->left->inferredType->sources ?: []; + $either_tainted = $stmt->left->inferredType->tainted; + } + + if (isset($stmt->right->inferredType)) { + $sources = array_merge($sources, $stmt->right->inferredType->sources ?: []); + $either_tainted = $either_tainted | $stmt->right->inferredType->tainted; + } + + if ($sources) { + $stmt->inferredType->sources = $sources; + } + + if ($either_tainted) { + $stmt->inferredType->tainted = $either_tainted; + } + } } elseif ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Coalesce) { $t_if_context = clone $context; @@ -578,6 +601,22 @@ class BinaryOpAnalyzer if ($result_type) { $stmt->inferredType = $result_type; } + + if ($codebase->taint && $stmt->inferredType) { + $sources = $stmt->left->inferredType->sources ?: []; + $either_tainted = $stmt->left->inferredType->tainted; + + $sources = array_merge($sources, $stmt->right->inferredType->sources ?: []); + $either_tainted = $either_tainted | $stmt->right->inferredType->tainted; + + if ($sources) { + $stmt->inferredType->sources = $sources; + } + + if ($either_tainted) { + $stmt->inferredType->tainted = $either_tainted; + } + } } elseif ($stmt instanceof PhpParser\Node\Expr\BinaryOp\BitwiseOr) { self::analyzeNonDivArithmeticOp( $statements_analyzer, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php index e381f2a00..4fe0e86e2 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php @@ -47,6 +47,7 @@ use function explode; use function array_search; use function array_keys; use function in_array; +use Psalm\Internal\Taint\TypeSource; /** * @internal @@ -1143,6 +1144,10 @@ class MethodCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\ $class_storage->parent_class ); + $return_type_candidate->sources = [ + new TypeSource(strtolower($method_id), new CodeLocation($source, $stmt->name)) + ]; + $return_type_location = $codebase->methods->getMethodReturnTypeLocation( $method_id, $secondary_return_type_location @@ -1284,6 +1289,15 @@ class MethodCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\ ); } + if ($codebase->taint && $method_id) { + $method_source = new TypeSource(strtolower($method_id), new CodeLocation($source, $stmt->name)); + + if ($codebase->taint->hasPreviousSource($method_source)) { + $return_type_candidate->tainted = 1; + $return_type_candidate->sources = [$method_source]; + } + } + if (!$return_type) { $return_type = $return_type_candidate; } else { diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php index 5525e6fe6..a126a8a21 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php @@ -31,6 +31,7 @@ use function strpos; use function is_string; use function strlen; use function substr; +use Psalm\Internal\Taint\TypeSource; /** * @internal @@ -989,6 +990,15 @@ class StaticCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\ } if ($return_type_candidate) { + if ($codebase->taint && $method_id) { + $method_source = new TypeSource(strtolower($method_id), new CodeLocation($source, $stmt->name)); + + if ($codebase->taint->hasPreviousSource($method_source)) { + $return_type_candidate->tainted = 1; + $return_type_candidate->sources = [$method_source]; + } + } + if (isset($stmt->inferredType)) { $stmt->inferredType = Type::combineUnionTypes($stmt->inferredType, $return_type_candidate); } else { diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php index e404db14a..32e44bf23 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php @@ -10,6 +10,7 @@ use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; use Psalm\Internal\Analyzer\TypeAnalyzer; use Psalm\Internal\Codebase\CallMap; +use Psalm\Internal\Taint\TypeSource; use Psalm\Internal\Type\TypeCombination; use Psalm\CodeLocation; use Psalm\Context; @@ -55,6 +56,7 @@ use function preg_replace; use function is_int; use function substr; use function array_merge; +use Psalm\Issue\TaintedInput; /** * @internal @@ -1740,7 +1742,9 @@ class CallAnalyzer $context, $function_param->by_ref, $function_param->is_variadic, - $arg->unpack + $arg->unpack, + $function_param->is_sink, + $function_param->assert_untainted ) === false) { return false; } @@ -2231,15 +2235,6 @@ class CallAnalyzer } /** - * @param StatementsAnalyzer $statements_analyzer - * @param Type\Union $input_type - * @param Type\Union $param_type - * @param string|null $cased_method_id - * @param int $argument_offset - * @param CodeLocation $code_location - * @param bool $by_ref - * @param bool $variadic - * * @return null|false */ public static function checkFunctionArgumentType( @@ -2247,14 +2242,16 @@ class CallAnalyzer Type\Union $input_type, Type\Union $param_type, ?Type\Union $signature_param_type, - $cased_method_id, + ?string $cased_method_id, int $argument_offset, CodeLocation $code_location, PhpParser\Node\Expr $input_expr, Context $context, bool $by_ref = false, bool $variadic = false, - bool $unpack = false + bool $unpack = false, + bool $is_sink = false, + bool $assert_untainted = false ) { $codebase = $statements_analyzer->getCodebase(); @@ -2379,6 +2376,98 @@ class CallAnalyzer $input_type = $union_comparison_results->replacement_union_type; } + if ($codebase->taint && $cased_method_id) { + $method_source = TypeSource::getForMethodArgument($cased_method_id, $argument_offset, $code_location); + + $has_previous_sink = $codebase->taint->hasPreviousSink($method_source); + + if (($is_sink || $has_previous_sink) + && $input_type->sources + ) { + $all_possible_sinks = []; + + foreach ($input_type->sources as $source) { + if ($codebase->taint->hasExistingSink($source)) { + continue; + } + + $all_possible_sinks[] = $source; + + if (strpos($source->id, '::') && strpos($source->id, '#')) { + list($fq_classlike_name, $method_name) = explode('::', $source->id); + + $method_name_parts = explode('#', $method_name); + + $method_name = strtolower($method_name_parts[0]); + + $class_storage = $codebase->classlike_storage_provider->get($fq_classlike_name); + + foreach ($class_storage->dependent_classlikes as $dependent_classlike => $_) { + $all_possible_sinks[] = TypeSource::getForMethodArgument( + $dependent_classlike . '::' . $method_name, + (int) $method_name_parts[1] - 1, + $code_location + ); + } + + if (isset($class_storage->overridden_method_ids[$method_name])) { + foreach ($class_storage->overridden_method_ids[$method_name] as $parent_method_id) { + $all_possible_sinks[] = TypeSource::getForMethodArgument( + $parent_method_id, + (int) $method_name_parts[1] - 1, + $code_location + ); + } + } + } + } + + $codebase->taint->addSinks( + $statements_analyzer, + $all_possible_sinks, + $code_location, + $method_source + ); + } + + if ($is_sink && $input_type->tainted) { + if (IssueBuffer::accepts( + new TaintedInput( + 'in path ' . $codebase->taint->getPredecessorPath($method_source) + . ' out path ' . $codebase->taint->getSuccessorPath($method_source), + $code_location + ), + $statements_analyzer->getSuppressedIssues() + )) { + // fall through + } + } elseif ($input_type->sources) { + foreach ($input_type->sources as $type_source) { + if ($codebase->taint->hasPreviousSource($type_source) || $input_type->tainted) { + $codebase->taint->addSources( + $statements_analyzer, + [$method_source], + $code_location, + $type_source + ); + } + } + } elseif ($input_type->tainted) { + throw new \UnexpectedValueException( + 'sources should exist for tainted var in ' + . $statements_analyzer->getFileName() . ':' + . $input_expr->getLine() + ); + } + + if ($assert_untainted) { + $input_type = clone $input_type; + $replace_input_type = true; + $input_type->tainted = null; + $input_type->sources = []; + } + } + if ($type_match_found && $param_type->hasCallableType() ) { @@ -2760,19 +2849,27 @@ class CallAnalyzer ); if ($var_id) { - $input_type = clone $input_type; + $was_cloned = false; if ($input_type->isNullable() && !$param_type->isNullable()) { + $input_type = clone $input_type; + $was_cloned = true; $input_type->removeType('null'); } if ($input_type->getId() === $param_type->getId()) { + if (!$was_cloned) { + $was_cloned = true; + $input_type = clone $input_type; + } + $input_type->from_docblock = false; foreach ($input_type->getTypes() as $atomic_type) { $atomic_type->from_docblock = false; } } elseif ($input_type->hasMixed() && $signature_param_type) { + $was_cloned = true; $input_type = clone $signature_param_type; if ($input_type->isNullable()) { @@ -2780,7 +2877,9 @@ class CallAnalyzer } } - $context->removeVarFromConflictingClauses($var_id, null, $statements_analyzer); + if ($was_cloned) { + $context->removeVarFromConflictingClauses($var_id, null, $statements_analyzer); + } if ($unpack) { $input_type = new Type\Union([ diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php index 0bffe5d8e..b45edfc43 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php @@ -51,6 +51,7 @@ use function strtolower; use function in_array; use function is_int; use function preg_match; +use Psalm\Internal\Taint\TypeSource; /** * @internal @@ -160,6 +161,13 @@ class ArrayFetchAnalyzer null ); + if ($array_var_id === '$_GET' || $array_var_id === '$_POST') { + $stmt->inferredType->tainted = Type\Union::TAINTED; + $stmt->inferredType->sources = [ + new TypeSource('$_GET', new CodeLocation($statements_analyzer->getSource(), $stmt)) + ]; + } + if ($context->inside_isset && $stmt->dim && isset($stmt->dim->inferredType) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/PropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/PropertyFetchAnalyzer.php index 4b17d1b47..e14330257 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/PropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/PropertyFetchAnalyzer.php @@ -39,6 +39,7 @@ use function array_reverse; use function array_keys; use function count; use function explode; +use Psalm\Internal\Taint\TypeSource; /** * @internal @@ -173,6 +174,18 @@ class PropertyFetchAnalyzer $property_id = $lhs_type_part->value . '::$' . $stmt->name->name; + if ($codebase->taint) { + $method_source = new TypeSource( + $property_id, + new CodeLocation($statements_analyzer, $stmt->name) + ); + + if ($codebase->taint->hasPreviousSource($method_source)) { + $stmt->inferredType->tainted = 1; + $stmt->inferredType->sources = [$method_source]; + } + } + $codebase->properties->propertyExists( $property_id, false, @@ -783,6 +796,15 @@ class PropertyFetchAnalyzer } } + if ($codebase->taint) { + $method_source = new TypeSource($property_id, new CodeLocation($statements_analyzer, $stmt->name)); + + if ($codebase->taint->hasPreviousSource($method_source)) { + $class_property_type->tainted = 1; + $class_property_type->sources = [$method_source]; + } + } + if (isset($stmt->inferredType)) { $stmt->inferredType = Type::combineUnionTypes($class_property_type, $stmt->inferredType); } else { diff --git a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php index fe858b9fd..be8352194 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php @@ -523,6 +523,11 @@ class ExpressionAnalyzer } $stmt->inferredType = Type::getString(); + + if (isset($stmt->expr->inferredType) && $stmt->expr->inferredType->tainted) { + $stmt->inferredType->tainted = $stmt->expr->inferredType->tainted; + $stmt->inferredType->sources = $stmt->expr->inferredType->sources; + } } elseif ($stmt instanceof PhpParser\Node\Expr\Cast\Object_) { if (self::analyze($statements_analyzer, $stmt->expr, $context) === false) { return false; @@ -1064,6 +1069,8 @@ class ExpressionAnalyzer $fleshed_out_type->by_ref = $return_type->by_ref; $fleshed_out_type->initialized = $return_type->initialized; $fleshed_out_type->had_template = $return_type->had_template; + $fleshed_out_type->sources = $return_type->sources; + $fleshed_out_type->tainted = $return_type->tainted; return $fleshed_out_type; } diff --git a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php index fd10ce876..1d3729424 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php @@ -12,6 +12,7 @@ use Psalm\Internal\Analyzer\TypeAnalyzer; use Psalm\CodeLocation; use Psalm\Context; use Psalm\Exception\DocblockParseException; +use Psalm\Internal\Taint\TypeSource; use Psalm\Issue\FalsableReturnStatement; use Psalm\Issue\InvalidDocblock; use Psalm\Issue\InvalidReturnStatement; @@ -20,10 +21,12 @@ use Psalm\Issue\MixedReturnStatement; use Psalm\Issue\MixedReturnTypeCoercion; use Psalm\Issue\NoValue; use Psalm\Issue\NullableReturnStatement; +use Psalm\Issue\TaintedInput; use Psalm\IssueBuffer; use Psalm\Type; use function explode; use function strtolower; +use UnexpectedValueException; /** * @internal @@ -155,15 +158,52 @@ class ReturnAnalyzer $cased_method_id = $source->getCorrectlyCasedMethodId(); if ($stmt->expr) { - if ($storage->return_type && !$storage->return_type->hasMixed()) { - $inferred_type = ExpressionAnalyzer::fleshOutType( - $codebase, - $stmt->inferredType, - $source->getFQCLN(), - $source->getFQCLN(), - $source->getParentFQCLN() + $inferred_type = ExpressionAnalyzer::fleshOutType( + $codebase, + $stmt->inferredType, + $source->getFQCLN(), + $source->getFQCLN(), + $source->getParentFQCLN() + ); + + if ($codebase->taint) { + $method_source = new TypeSource( + strtolower($cased_method_id), + new CodeLocation($source, $stmt->expr) ); + if ($codebase->taint->hasPreviousSink($method_source)) { + if ($inferred_type->sources) { + $codebase->taint->addSinks( + $statements_analyzer, + $inferred_type->sources, + new CodeLocation($source, $stmt->expr), + $method_source + ); + } + } + + if ($inferred_type->sources) { + foreach ($inferred_type->sources as $type_source) { + if ($codebase->taint->hasPreviousSource($type_source) || $inferred_type->tainted) { + $codebase->taint->addSources( + $statements_analyzer, + [$method_source], + new CodeLocation($source, $stmt->expr), + $type_source + ); + } + } + } elseif ($inferred_type->tainted) { + throw new \UnexpectedValueException( + 'sources should exist for tainted var in ' + . $statements_analyzer->getFileName() . ':' + . $stmt->getLine() + ); + } + } + + if ($storage->return_type && !$storage->return_type->hasMixed()) { $local_return_type = $source->getLocalReturnType($storage->return_type); if ($storage instanceof \Psalm\Storage\MethodStorage) { diff --git a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php index e733d4d6a..ffc24d559 100644 --- a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php @@ -514,7 +514,8 @@ class StatementsAnalyzer extends SourceAnalyzer implements StatementsSource (int)$i, new CodeLocation($this->getSource(), $expr), $expr, - $context + $context, + true ) === false) { return false; } diff --git a/src/Psalm/Internal/Codebase/Analyzer.php b/src/Psalm/Internal/Codebase/Analyzer.php index c1c55df5f..e17a9955c 100644 --- a/src/Psalm/Internal/Codebase/Analyzer.php +++ b/src/Psalm/Internal/Codebase/Analyzer.php @@ -68,7 +68,8 @@ use function usort; * class_locations: array>, * class_method_locations: array>, * class_property_locations: array>, - * possible_method_param_types: array> + * possible_method_param_types: array>, + * taint_data: ?\Psalm\Internal\Codebase\Taint * } */ @@ -240,7 +241,6 @@ class Analyzer { $this->loadCachedResults($project_analyzer); - $filetype_analyzers = $this->config->getFiletypeAnalyzers(); $codebase = $project_analyzer->getCodebase(); if ($alter_code) { @@ -254,6 +254,60 @@ class Analyzer } ); + $this->doAnalysis($project_analyzer, $pool_size); + + if ($codebase->taint) { + $i = 0; + while ($codebase->taint->hasNewSinksAndSources() && ++$i <= 4) { + $project_analyzer->progress->write("\n\n" . 'Found tainted inputs, reanalysing' . "\n\n"); + + $codebase->taint->clearNewSinksAndSources(); + + $this->doAnalysis($project_analyzer, $pool_size, true); + } + } + + $this->progress->finish(); + + if ($codebase->find_unused_code + && ($project_analyzer->full_run || $codebase->find_unused_code === 'always') + ) { + $project_analyzer->checkClassReferences(); + } + + $scanned_files = $codebase->scanner->getScannedFiles(); + $codebase->file_reference_provider->setAnalyzedMethods($this->analyzed_methods); + $codebase->file_reference_provider->setFileMaps($this->getFileMaps()); + $codebase->file_reference_provider->setTypeCoverage($this->mixed_counts); + $codebase->file_reference_provider->updateReferenceCache($codebase, $scanned_files); + + if ($codebase->diff_methods) { + $codebase->statements_provider->resetDiffs(); + } + + if ($alter_code) { + $this->progress->startAlteringFiles(); + + $project_analyzer->prepareMigration(); + + $files_to_update = $this->files_to_update !== null ? $this->files_to_update : $this->files_to_analyze; + + foreach ($files_to_update as $file_path) { + $this->updateFile($file_path, $project_analyzer->dry_run); + } + + $project_analyzer->migrateCode(); + } + } + + private function doAnalysis(ProjectAnalyzer $project_analyzer, int $pool_size, bool $rerun = false) : void + { + $this->progress->start(count($this->files_to_analyze)); + + $codebase = $project_analyzer->getCodebase(); + + $filetype_analyzers = $this->config->getFiletypeAnalyzers(); + $analysis_worker = /** * @param int $_ @@ -274,8 +328,6 @@ class Analyzer return $this->getFileIssues($file_path); }; - $this->progress->start(count($this->files_to_analyze)); - $task_done_closure = /** * @param array $issues @@ -329,7 +381,7 @@ class Analyzer }, $analysis_worker, /** @return WorkerData */ - function () { + function () use ($rerun) { $project_analyzer = ProjectAnalyzer::getInstance(); $codebase = $project_analyzer->getCodebase(); $analyzer = $codebase->analyzer; @@ -340,21 +392,22 @@ class Analyzer // @codingStandardsIgnoreStart return [ 'issues' => IssueBuffer::getIssuesData(), - 'file_references_to_classes' => $file_reference_provider->getAllFileReferencesToClasses(), - 'file_references_to_class_members' => $file_reference_provider->getAllFileReferencesToClassMembers(), - 'method_references_to_class_members' => $file_reference_provider->getAllMethodReferencesToClassMembers(), - 'file_references_to_missing_class_members' => $file_reference_provider->getAllFileReferencesToMissingClassMembers(), - 'method_references_to_missing_class_members' => $file_reference_provider->getAllMethodReferencesToMissingClassMembers(), - 'method_param_uses' => $file_reference_provider->getAllMethodParamUses(), - 'mixed_member_names' => $analyzer->getMixedMemberNames(), - 'file_manipulations' => FileManipulationBuffer::getAll(), - 'mixed_counts' => $analyzer->getMixedCounts(), - 'analyzed_methods' => $analyzer->getAnalyzedMethods(), - 'file_maps' => $analyzer->getFileMaps(), - 'class_locations' => $file_reference_provider->getAllClassLocations(), - 'class_method_locations' => $file_reference_provider->getAllClassMethodLocations(), - 'class_property_locations' => $file_reference_provider->getAllClassPropertyLocations(), - 'possible_method_param_types' => $analyzer->getPossibleMethodParamTypes(), + 'file_references_to_classes' => $rerun ? [] : $file_reference_provider->getAllFileReferencesToClasses(), + 'file_references_to_class_members' => $rerun ? [] : $file_reference_provider->getAllFileReferencesToClassMembers(), + 'method_references_to_class_members' => $rerun ? [] : $file_reference_provider->getAllMethodReferencesToClassMembers(), + 'file_references_to_missing_class_members' => $rerun ? [] : $file_reference_provider->getAllFileReferencesToMissingClassMembers(), + 'method_references_to_missing_class_members' => $rerun ? [] : $file_reference_provider->getAllMethodReferencesToMissingClassMembers(), + 'method_param_uses' => $rerun ? [] : $file_reference_provider->getAllMethodParamUses(), + 'mixed_member_names' => $rerun ? [] : $analyzer->getMixedMemberNames(), + 'file_manipulations' => $rerun ? [] : FileManipulationBuffer::getAll(), + 'mixed_counts' => $rerun ? [] : $analyzer->getMixedCounts(), + 'analyzed_methods' => $rerun ? [] : $analyzer->getAnalyzedMethods(), + 'file_maps' => $rerun ? [] : $analyzer->getFileMaps(), + 'class_locations' => $rerun ? [] : $file_reference_provider->getAllClassLocations(), + 'class_method_locations' => $rerun ? [] : $file_reference_provider->getAllClassMethodLocations(), + 'class_property_locations' => $rerun ? [] : $file_reference_provider->getAllClassPropertyLocations(), + 'possible_method_param_types' => $rerun ? [] : $analyzer->getPossibleMethodParamTypes(), + 'taint_data' => $codebase->taint, ]; // @codingStandardsIgnoreEnd }, @@ -374,6 +427,14 @@ class Analyzer foreach ($forked_pool_data as $pool_data) { IssueBuffer::addIssues($pool_data['issues']); + if ($codebase->taint && $pool_data['taint_data']) { + $codebase->taint->addThreadData($pool_data['taint_data']); + } + + if ($rerun) { + continue; + } + foreach ($pool_data['issues'] as $issue_data) { $codebase->file_reference_provider->addIssue($issue_data['file_path'], $issue_data); } @@ -470,40 +531,6 @@ class Analyzer $codebase->file_reference_provider->addIssue($issue_data['file_path'], $issue_data); } } - - $this->progress->finish(); - - $codebase = $project_analyzer->getCodebase(); - - if ($codebase->find_unused_code - && ($project_analyzer->full_run || $codebase->find_unused_code === 'always') - ) { - $project_analyzer->checkClassReferences(); - } - - $scanned_files = $codebase->scanner->getScannedFiles(); - $codebase->file_reference_provider->setAnalyzedMethods($this->analyzed_methods); - $codebase->file_reference_provider->setFileMaps($this->getFileMaps()); - $codebase->file_reference_provider->setTypeCoverage($this->mixed_counts); - $codebase->file_reference_provider->updateReferenceCache($codebase, $scanned_files); - - if ($codebase->diff_methods) { - $codebase->statements_provider->resetDiffs(); - } - - if ($alter_code) { - $this->progress->startAlteringFiles(); - - $project_analyzer->prepareMigration(); - - $files_to_update = $this->files_to_update !== null ? $this->files_to_update : $this->files_to_analyze; - - foreach ($files_to_update as $file_path) { - $this->updateFile($file_path, $project_analyzer->dry_run); - } - - $project_analyzer->migrateCode(); - } } /** diff --git a/src/Psalm/Internal/Codebase/Reflection.php b/src/Psalm/Internal/Codebase/Reflection.php index 6ecbd0255..56ffe85f4 100644 --- a/src/Psalm/Internal/Codebase/Reflection.php +++ b/src/Psalm/Internal/Codebase/Reflection.php @@ -245,8 +245,9 @@ class Reflection $storage->is_static = $method->isStatic(); $storage->abstract = $method->isAbstract(); - $class_storage->declaring_method_ids[$method_name] = - $declaring_class->name . '::' . strtolower((string)$method->getName()); + $declaring_method_id = $declaring_class->name . '::' . strtolower((string)$method->getName()); + + $class_storage->declaring_method_ids[$method_name] = $declaring_method_id; $class_storage->inheritable_method_ids[$method_name] = $class_storage->declaring_method_ids[$method_name]; $class_storage->appearing_method_ids[$method_name] = $class_storage->declaring_method_ids[$method_name]; @@ -261,10 +262,14 @@ class Reflection if ($callables && $callables[0]->params !== null && $callables[0]->return_type !== null) { $storage->params = []; - foreach ($callables[0]->params as $param) { + foreach ($callables[0]->params as $i => $param) { if ($param->type) { $param->type->queueClassLikesForScanning($this->codebase); } + + if ($declaring_method_id === 'PDO::exec' && $i === 0) { + $param->is_sink = true; + } } $storage->params = $callables[0]->params; diff --git a/src/Psalm/Internal/Codebase/Taint.php b/src/Psalm/Internal/Codebase/Taint.php new file mode 100644 index 000000000..f35b75acf --- /dev/null +++ b/src/Psalm/Internal/Codebase/Taint.php @@ -0,0 +1,195 @@ + + */ + private $new_sinks = []; + + /** + * @var array + */ + private $new_sources = []; + + /** + * @var array + */ + private $previous_sinks = []; + + /** + * @var array + */ + private $previous_sources = []; + + /** + * @var array + */ + private $archived_sinks = []; + + /** + * @var array + */ + private $archived_sources = []; + + public function hasExistingSink(TypeSource $source) : ?TypeSource + { + return $this->archived_sinks[$source->id] ?? null; + } + + public function hasPreviousSink(TypeSource $source) : bool + { + return isset($this->previous_sinks[$source->id]); + } + + public function hasPreviousSource(TypeSource $source) : bool + { + return isset($this->previous_sources[$source->id]); + } + + public function hasExistingSource(TypeSource $source) : ?TypeSource + { + return $this->archived_sources[$source->id] ?? null; + } + + /** + * @param array $sources + */ + public function addSources( + StatementsAnalyzer $statements_analyzer, + array $sources, + \Psalm\CodeLocation $code_location, + ?TypeSource $previous_source + ) : void { + foreach ($sources as $source) { + if ($this->hasExistingSource($source)) { + continue; + } + + if ($this->hasExistingSink($source)) { + if (IssueBuffer::accepts( + new TaintedInput( + ($previous_source ? 'in path ' . $this->getPredecessorPath($previous_source) : '') + . ' out path ' . $this->getSuccessorPath($source), + $code_location + ), + $statements_analyzer->getSuppressedIssues() + )) { + // fall through + } + } + + $this->new_sources[$source->id] = $previous_source; + } + } + + public function getPredecessorPath(TypeSource $source) : string + { + $source_descriptor = $source->id + . ($source->code_location ? ' (' . $source->code_location->getShortSummary() . ')' : ''); + + if ($previous_source = $this->new_sources[$source->id] ?? $this->archived_sources[$source->id] ?? null) { + if ($previous_source === $source) { + throw new \UnexpectedValueException('bad'); + } + + return $this->getPredecessorPath($previous_source) . ' -> ' . $source_descriptor; + } + + return $source_descriptor; + } + + public function getSuccessorPath(TypeSource $source) : string + { + $source_descriptor = $source->id + . ($source->code_location ? ' (' . $source->code_location->getShortSummary() . ')' : ''); + + if ($next_source = $this->new_sinks[$source->id] ?? $this->archived_sinks[$source->id] ?? null) { + return $source_descriptor . ' -> ' . $this->getSuccessorPath($next_source); + } + + return $source_descriptor; + } + + /** + * @param array $sources + */ + public function addSinks( + StatementsAnalyzer $statements_analyzer, + array $sources, + \Psalm\CodeLocation $code_location, + ?TypeSource $previous_source + ) : void { + foreach ($sources as $source) { + if ($this->hasExistingSink($source)) { + continue; + } + + if ($this->hasExistingSource($source)) { + if (IssueBuffer::accepts( + new TaintedInput( + 'in path ' . $this->getPredecessorPath($source) + . ($previous_source ? ' out path ' . $this->getSuccessorPath($previous_source) : ''), + $code_location + ), + $statements_analyzer->getSuppressedIssues() + )) { + // fall through + } + } + + $this->new_sinks[$source->id] = $previous_source; + } + } + + public function hasNewSinksAndSources() : bool + { + return $this->new_sinks && $this->new_sources; + } + + public function addThreadData(self $taint) : void + { + $this->new_sinks = array_merge( + $this->new_sinks, + $taint->new_sinks + ); + + $this->new_sources = array_merge( + $this->new_sources, + $taint->new_sources + ); + } + + public function clearNewSinksAndSources() : void + { + $this->archived_sinks = array_merge( + $this->archived_sinks, + $this->new_sinks + ); + + $this->previous_sinks = $this->new_sinks; + + $this->new_sinks = []; + + $this->archived_sources = array_merge( + $this->archived_sources, + $this->new_sources + ); + + $this->previous_sources = $this->new_sources; + + $this->new_sources = []; + } +} diff --git a/src/Psalm/Internal/Scanner/FunctionDocblockComment.php b/src/Psalm/Internal/Scanner/FunctionDocblockComment.php index 95b370b8d..94c15869f 100644 --- a/src/Psalm/Internal/Scanner/FunctionDocblockComment.php +++ b/src/Psalm/Internal/Scanner/FunctionDocblockComment.php @@ -41,6 +41,16 @@ class FunctionDocblockComment */ public $params_out = []; + /** + * @var array + */ + public $taint_sink_params = []; + + /** + * @var array + */ + public $assert_untainted_params = []; + /** * @var array */ diff --git a/src/Psalm/Internal/Taint/TypeSource.php b/src/Psalm/Internal/Taint/TypeSource.php new file mode 100644 index 000000000..bd0eed07c --- /dev/null +++ b/src/Psalm/Internal/Taint/TypeSource.php @@ -0,0 +1,33 @@ +id = $id; + $this->code_location = $code_location; + } + + public static function getForMethodArgument( + string $method_id, + int $argument_offset, + ?CodeLocation $code_location + ) : self { + return new self(\strtolower($method_id . '#' . ($argument_offset + 1)), $code_location); + } + + public function __toString() + { + return $this->id; + } +} diff --git a/src/Psalm/Internal/Visitor/ReflectorVisitor.php b/src/Psalm/Internal/Visitor/ReflectorVisitor.php index caedede15..d69e93276 100644 --- a/src/Psalm/Internal/Visitor/ReflectorVisitor.php +++ b/src/Psalm/Internal/Visitor/ReflectorVisitor.php @@ -2283,6 +2283,26 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse } } + foreach ($docblock_info->taint_sink_params as $taint_sink_param) { + $param_name = substr($taint_sink_param['name'], 1); + + foreach ($storage->params as $param_storage) { + if ($param_storage->name === $param_name) { + $param_storage->is_sink = true; + } + } + } + + foreach ($docblock_info->assert_untainted_params as $untainted_assert_param) { + $param_name = substr($untainted_assert_param['name'], 1); + + foreach ($storage->params as $param_storage) { + if ($param_storage->name === $param_name) { + $param_storage->assert_untainted = true; + } + } + } + if ($docblock_info->template_typeofs) { foreach ($docblock_info->template_typeofs as $template_typeof) { foreach ($storage->params as $param) { diff --git a/src/Psalm/Issue/TaintedInput.php b/src/Psalm/Issue/TaintedInput.php new file mode 100644 index 000000000..419b397ee --- /dev/null +++ b/src/Psalm/Issue/TaintedInput.php @@ -0,0 +1,6 @@ +failed_reconciliation = true; } + + if ($type_1->tainted || $type_2->tainted) { + $combined_type->tainted = $type_1->tainted & $type_2->tainted; + } } if ($type_1->possibly_undefined || $type_2->possibly_undefined) { $combined_type->possibly_undefined = true; } + if ($type_1->sources || $type_2->sources) { + $combined_type->sources = \array_unique( + array_merge($type_1->sources ?: [], $type_2->sources ?: []) + ); + } + return $combined_type; } diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index b4a60ee12..169874030 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -31,6 +31,10 @@ use function substr; class Union { + const TAINTED = 1; + const TAINTED_MYSQL_SAFE = 2; + const TAINTED_HTML_SAFE = 4; + /** * @var array */ @@ -141,6 +145,16 @@ class Union /** @var null|string */ private $id; + /** + * @var ?int + */ + public $tainted = null; + + /** + * @var ?array<\Psalm\Internal\Taint\TypeSource> + */ + public $sources; + /** * Constructs an Union instance * diff --git a/src/psalm.php b/src/psalm.php index 44e249be3..8328cae90 100644 --- a/src/psalm.php +++ b/src/psalm.php @@ -62,6 +62,7 @@ $valid_long_options = [ 'shepherd::', 'no-progress', 'include-php-versions', // used for baseline + 'track-tainted-input' ]; gc_collect_cycles(); @@ -127,7 +128,7 @@ array_map( if (!array_key_exists('use-ini-defaults', $options)) { ini_set('display_errors', '1'); ini_set('display_startup_errors', '1'); - ini_set('memory_limit', (string) (4 * 1024 * 1024 * 1024)); + ini_set('memory_limit', (string) (8 * 1024 * 1024 * 1024)); } if (array_key_exists('help', $options)) { @@ -508,6 +509,10 @@ if ($config->find_unused_variables) { $project_analyzer->getCodebase()->reportUnusedVariables(); } +if (isset($options['track-tainted-input'])) { + $project_analyzer->trackTaintedInputs(); +} + /** @var string $plugin_path */ foreach ($plugins as $plugin_path) { $config->addPluginPath($plugin_path); diff --git a/tests/DocumentationTest.php b/tests/DocumentationTest.php index db21027db..97ed054bb 100644 --- a/tests/DocumentationTest.php +++ b/tests/DocumentationTest.php @@ -110,6 +110,7 @@ class DocumentationTest extends TestCase $code_blocks['UnrecognizedExpression'] = true; $code_blocks['UnrecognizedStatement'] = true; $code_blocks['PluginIssue'] = true; + $code_blocks['TaintedInput'] = true; // these are deprecated $code_blocks['TypeCoercion'] = true; diff --git a/tests/Progress/EchoProgress.php b/tests/Progress/EchoProgress.php index 200059998..4954aa338 100644 --- a/tests/Progress/EchoProgress.php +++ b/tests/Progress/EchoProgress.php @@ -5,7 +5,7 @@ use Psalm\Progress\DefaultProgress; class EchoProgress extends DefaultProgress { - protected function write(string $message): void + public function write(string $message): void { echo $message; } diff --git a/tests/TaintTest.php b/tests/TaintTest.php new file mode 100644 index 000000000..b8d277d41 --- /dev/null +++ b/tests/TaintTest.php @@ -0,0 +1,534 @@ +expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('TaintedInput'); + + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'getUserId(); + } + + public function deleteUser(PDO $pdo) : void { + $userId = $this->getAppendedUserId(); + $pdo->exec("delete from users where user_id = " . $userId); + } + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testTaintedInputDirectly() + { + $this->expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('TaintedInput'); + + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'exec("delete from users where user_id = " . $userId); + } + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testTaintedInputDirectlySuppressed() + { + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'exec("delete from users where user_id = " . $userId); + } + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testTaintedInputDirectlySuppressedWithOtherUse() + { + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'exec("delete from users where user_id = " . $userId); + } + + public function deleteUserSafer(PDOWrapper $pdo) : void { + $userId = $this->getSafeId(); + $pdo->exec("delete from users where user_id = " . $userId); + } + + public function getSafeId() : string { + return "5"; + } + } + + class PDOWrapper { + /** + * @psalm-taint-sink $sql + */ + public function exec(string $sql) : void {} + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testTaintedInputFromReturnTypeWithBranch() + { + $this->expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('TaintedInput'); + + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'getUserId(); + + if (rand(0, 1)) { + $userId .= "aaa"; + } else { + $userId .= "bb"; + } + + return $userId; + } + + public function deleteUser(PDO $pdo) : void { + $userId = $this->getAppendedUserId(); + $pdo->exec("delete from users where user_id = " . $userId); + } + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testSinkAnnotation() + { + $this->expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('TaintedInput'); + + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'getUserId(); + } + + public function deleteUser(PDOWrapper $pdo) : void { + $userId = $this->getAppendedUserId(); + $pdo->exec("delete from users where user_id = " . $userId); + } + } + + class PDOWrapper { + /** + * @psalm-taint-sink $sql + */ + public function exec(string $sql) : void {} + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testTaintedInputFromParam() + { + $this->expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('TaintedInput - somefile.php:8:32 - in path $_GET (somefile.php:4:41) -> a::getuserid (somefile.php:8:48) out path a::getappendeduserid (somefile.php:8:32) -> a::deleteuser#2 (somefile.php:13:49) -> pdo::exec#1 (somefile.php:17:36)'); + + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'getUserId(); + } + + public function doDelete(PDO $pdo) : void { + $userId = $this->getAppendedUserId(); + $this->deleteUser($pdo, $userId); + } + + public function deleteUser(PDO $pdo, string $userId) : void { + $pdo->exec("delete from users where user_id = " . $userId); + } + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testTaintedInputToParam() + { + $this->expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('TaintedInput'); + + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'deleteUser( + $pdo, + $this->getAppendedUserId((string) $_GET["user_id"]) + ); + } + + public function getAppendedUserId(string $user_id) : string { + return "aaa" . $user_id; + } + + public function deleteUser(PDO $pdo, string $userId) : void { + $pdo->exec("delete from users where user_id = " . $userId); + } + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testTaintedInputToParamAfterAssignment() + { + $this->expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('TaintedInput'); + + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'deleteUser( + $pdo, + $this->getAppendedUserId((string) $_GET["user_id"]) + ); + } + + public function getAppendedUserId(string $user_id) : string { + return "aaa" . $user_id; + } + + public function deleteUser(PDO $pdo, string $userId) : void { + $userId2 = $userId; + $pdo->exec("delete from users where user_id = " . $userId2); + } + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testTaintedInputToParamButSafe() + { + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'deleteUser( + $pdo, + $this->getAppendedUserId((string) $_GET["user_id"]) + ); + } + + public function getAppendedUserId(string $user_id) : string { + return "aaa" . $user_id; + } + + public function deleteUser(PDO $pdo, string $userId) : void { + $userId2 = strlen($userId); + $pdo->exec("delete from users where user_id = " . $userId2); + } + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testTaintedInputToParamAlternatePath() + { + $this->expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('TaintedInput - somefile.php:7:29 - in path $_GET (somefile.php:7:63) -> a::getappendeduserid#1 (somefile.php:11:62) -> a::getappendeduserid (somefile.php:7:36) out path a::deleteuser#3 (somefile.php:7:29) -> pdo::exec#1 (somefile.php:23:40)'); + + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'deleteUser( + $pdo, + self::doFoo(), + $this->getAppendedUserId((string) $_GET["user_id"]) + ); + } + + public function getAppendedUserId(string $user_id) : string { + return "aaa" . $user_id; + } + + public static function doFoo() : string { + return "hello"; + } + + public function deleteUser(PDO $pdo, string $userId, string $userId2) : void { + $pdo->exec("delete from users where user_id = " . $userId); + + if (rand(0, 1)) { + $pdo->exec("delete from users where user_id = " . $userId2); + } + } + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testTaintedInParentLoader() + { + $this->expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('TaintedInput - somefile.php:24:47 - in path $_GET (somefile.php:28:39) -> c::foo#1 (somefile.php:23:48) out path agrandchild::loadfull#1 (somefile.php:24:47) -> a::loadpartial#1 (somefile.php:6:45) -> pdo::exec#1 (somefile.php:16:40)'); + + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'exec("select * from foo where bar = " . $sink); + } + } + + class AGrandChild extends AChild {} + + class C { + public function foo(string $user_id) : void { + AGrandChild::loadFull($user_id); + } + } + + (new C)->foo((string) $_GET["user_id"]);' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testValidatedInputFromParam() + { + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'getUserId(); + validateUserId($userId); + $this->deleteUser($pdo, $userId); + } + + public function deleteUser(PDO $pdo, string $userId) : void { + $pdo->exec("delete from users where user_id = " . $userId); + } + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testUntaintedInput() + { + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'getUserId(); + } + + public function deleteUser(PDO $pdo) : void { + $userId = $this->getAppendedUserId(); + $pdo->exec("delete from users where user_id = " . $userId); + } + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } + + /** + * @return void + */ + public function testTaintedInputFromProperty() + { + $this->expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('TaintedInput'); + + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'userId = (string) $_GET["user_id"]; + } + + public function getAppendedUserId() : string { + return "aaaa" . $this->userId; + } + + public function doDelete(PDO $pdo) : void { + $userId = $this->getAppendedUserId(); + $this->deleteUser($pdo, $userId); + } + + public function deleteUser(PDO $pdo, string $userId) : void { + $pdo->exec("delete from users where user_id = " . $userId); + } + }' + ); + + $this->analyzeFile('somefile.php', new Context()); + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php index 102f9415d..2b330527a 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -111,6 +111,14 @@ class TestCase extends BaseTestCase $codebase->config->shortenFileName($file_path) ); $file_analyzer->analyze($context); + + if ($codebase->taint) { + while ($codebase->taint->hasNewSinksAndSources()) { + $codebase->taint->clearNewSinksAndSources(); + + $file_analyzer->analyze($context); + } + } } /**