Skip to content

Commit

Permalink
Add framework for taint analysis to Psalm
Browse files Browse the repository at this point in the history 
Ref vimeo#611
  • Loading branch information
@muglug @2e3s
muglug authored and 2e3s committed Sep 29, 2019
1 parent 5a8a689 commit f9de73d
Show file tree
Hide file tree
Showing 35 changed files with 1,298 additions and 83 deletions.
1 change: 1 addition & 0 deletions config.xsd
View file
Expand Up @@ -309,6 +309,7 @@
<xs:element name="RedundantConditionGivenDocblockType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="ReferenceConstraintViolation" type="IssueHandlerType" minOccurs="0" />
<xs:element name="ReservedWord" type="IssueHandlerType" minOccurs="0" />
<xs:element name="TaintedInput" type="IssueHandlerType" minOccurs="0" />
<xs:element name="TraitMethodSignatureMismatch" type="IssueHandlerType" minOccurs="0" />
<xs:element name="TooFewArguments" type="ArgumentIssueHandlerType" minOccurs="0" />
<xs:element name="TooManyArguments" type="ArgumentIssueHandlerType" minOccurs="0" />
Expand Down
4 changes: 4 additions & 0 deletions docs/running_psalm/issues.md
View file
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions src/Psalm/CodeLocation.php
View file
Expand Up @@ -404,4 +404,9 @@ public function getHash()
{
return (string) $this->file_start;
}

public function getShortSummary() : string
{
return $this->file_name . ':' . $this->getLineNumber() . ':' . $this->getColumn();
}
}
7 changes: 7 additions & 0 deletions src/Psalm/Codebase.php
View file
Expand Up @@ -170,6 +170,11 @@ class Codebase
*/
public $populator;

/**
* @var ?Internal\Codebase\Taint
*/
public $taint = null;

/**
* @var bool
*/
Expand Down Expand Up @@ -267,6 +272,8 @@ class Codebase
*/
public $php_minor_version = PHP_MINOR_VERSION;



public function __construct(
Config $config,
Providers $providers,
Expand Down
2 changes: 2 additions & 0 deletions src/Psalm/DocComment.php
View file
Expand Up @@ -150,6 +150,7 @@ public static function parse($docblock, $line_number = null, $preserve_format =
'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
)) {
Expand Down Expand Up @@ -271,6 +272,7 @@ public static function parsePreservingLength(\PhpParser\Comment\Doc $docblock)
'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
)) {
Expand Down
18 changes: 18 additions & 0 deletions src/Psalm/Internal/Analyzer/CommentAnalyzer.php
View file
Expand Up @@ -429,6 +429,24 @@ public static function extractFunctionDocblockInfo(PhpParser\Comment\Doc $commen
}
}

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);
Expand Down
1 change: 1 addition & 0 deletions src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php
View file
Expand Up @@ -39,6 +39,7 @@
use function substr;
use function count;
use function in_array;
use Psalm\Internal\Taint\TypeSource;

/**
* @internal
Expand Down
12 changes: 11 additions & 1 deletion src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
View file
Expand Up @@ -42,6 +42,7 @@
use function array_keys;
use function end;
use function array_diff;
use Psalm\Internal\Taint\TypeSource;

/**
* @internal
Expand Down Expand Up @@ -267,7 +268,7 @@ public function analyze(

$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(
Expand Down Expand Up @@ -591,6 +592,15 @@ function (FunctionLikeParameter $p) {
]);
}

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;

Expand Down
9 changes: 9 additions & 0 deletions src/Psalm/Internal/Analyzer/ProjectAnalyzer.php
View file
Expand Up @@ -82,6 +82,7 @@
use function substr_count;
use function array_map;
use function end;
use Psalm\Internal\Codebase\Taint;

/**
* @internal
Expand Down Expand Up @@ -549,6 +550,14 @@ public function checkClassReferences()
);
}

/**
* @return void
*/
public function trackTaintedInputs()
{
$this->codebase->taint = new Taint();
}

public function interpretRefactors() : void
{
if (!$this->codebase->alter_code) {
Expand Down
40 changes: 40 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php
View file
Expand Up @@ -41,6 +41,7 @@
use function in_array;
use function strtolower;
use function explode;
use Psalm\Internal\Taint\TypeSource;

/**
* @internal
Expand Down Expand Up @@ -455,6 +456,45 @@ public static function analyzeInstance(
}
}

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,
Expand Down
39 changes: 39 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php
View file
Expand Up @@ -329,6 +329,29 @@ function ($var_id) use ($original_vars_in_scope) {
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;

Expand Down Expand Up @@ -578,6 +601,22 @@ function (\Psalm\Internal\Clause $c) use ($mixed_var_ids) {
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,
Expand Down
14 changes: 14 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php
View file
Expand Up @@ -47,6 +47,7 @@
use function array_search;
use function array_keys;
use function in_array;
use Psalm\Internal\Taint\TypeSource;

/**
* @internal
Expand Down Expand Up @@ -1143,6 +1144,10 @@ function (PhpParser\Node\Arg $arg) {
$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
Expand Down Expand Up @@ -1284,6 +1289,15 @@ function (Assertion $assertion) use ($class_template_params) : Assertion {
);
}

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 {
Expand Down
10 changes: 10 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php
View file
Expand Up @@ -31,6 +31,7 @@
use function is_string;
use function strlen;
use function substr;
use Psalm\Internal\Taint\TypeSource;

/**
* @internal
Expand Down Expand Up @@ -989,6 +990,15 @@ function (Assertion $assertion) use ($found_generic_params) : Assertion {
}

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 {
Expand Down

0 comments on commit f9de73d

Please sign in to comment.