Skip to content

Commit

Permalink
Allow to implement custom taint type classes
Browse files Browse the repository at this point in the history 
Instead of just having a generic `TaintedCustom` for custom
taint - this change allows plugins/extensions to register their
own custom taint type classes.

Examples

```
TaintTypeRegistry::getType('html'); // returns \Psalm\Issue\TaintedHtml
TaintTypeRegistry::addType(
  'my-type',
  \Example\TaintedMyType::class,
  TaintKindGroup::GROUP_INPUT,
);
```
  • Loading branch information
@ohader
ohader committed Feb 23, 2024
1 parent eeffee7 commit 9766b44
Show file tree
Hide file tree
Showing 29 changed files with 326 additions and 173 deletions.
3 changes: 2 additions & 1 deletion docs/security_analysis/custom_taint_sources.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class BadSqlTainter implements AfterExpressionAnalysisInterface
$expr = $event->getExpr();
$statements_source = $event->getStatementsSource();
$codebase = $event->getCodebase();
$taint_type_registry = $codebase->config->taint_type_registry;
if ($expr instanceof PhpParser\Node\Expr\Variable
&& $expr->name === 'bad_data'
) {
Expand All @@ -63,7 +64,7 @@ class BadSqlTainter implements AfterExpressionAnalysisInterface
$codebase->addTaintSource(
$expr_type,
$expr_identifier,
TaintKindGroup::ALL_INPUT,
$taint_type_registry->resolveGroup(TaintKindGroup::GROUP_INPUT),
new CodeLocation($statements_source, $expr)
);
}
Expand Down
8 changes: 4 additions & 4 deletions src/Psalm/Codebase.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2423,7 +2423,7 @@ public function queueClassLikeForScanning(
public function addTaintSource(

Check failure on line 2423 in src/Psalm/Codebase.php

View workflow job for this annotation

GitHub Actions / Check backward compatibility 

Default parameter value for parameter $taints of Psalm\Codebase#addTaintSource() changed from array (
  0 => 'html',
  1 => 'has_quotes',
  2 => 'shell',
  3 => 'sql',
  4 => 'callable',
  5 => 'eval',
  6 => 'unserialize',
  7 => 'include',
  8 => 'ssrf',
  9 => 'ldap',
  10 => 'file',
  11 => 'header',
  12 => 'cookie',
) to NULL
Union $expr_type,
string $taint_id,
array $taints = TaintKindGroup::ALL_INPUT,
array $taints = null,
?CodeLocation $code_location = null
): Union {
if (!$this->taint_flow_graph) {
Expand All @@ -2435,7 +2435,7 @@ public function addTaintSource(
$taint_id,
$code_location,
null,
$taints,
$taints ?? $this->config->taint_type_registry->resolveGroup(TaintKindGroup::GROUP_INPUT),
);

$this->taint_flow_graph->addSource($source);
Expand All @@ -2449,7 +2449,7 @@ public function addTaintSource(
*/
public function addTaintSink(

Check failure on line 2450 in src/Psalm/Codebase.php

View workflow job for this annotation

GitHub Actions / Check backward compatibility 

Default parameter value for parameter $taints of Psalm\Codebase#addTaintSink() changed from array (
  0 => 'html',
  1 => 'has_quotes',
  2 => 'shell',
  3 => 'sql',
  4 => 'callable',
  5 => 'eval',
  6 => 'unserialize',
  7 => 'include',
  8 => 'ssrf',
  9 => 'ldap',
  10 => 'file',
  11 => 'header',
  12 => 'cookie',
) to NULL
string $taint_id,
array $taints = TaintKindGroup::ALL_INPUT,
array $taints = null,
?CodeLocation $code_location = null
): void {
if (!$this->taint_flow_graph) {
Expand All @@ -2461,7 +2461,7 @@ public function addTaintSink(
$taint_id,
$code_location,
null,
$taints,
$taints ?? $this->config->taint_type_registry->resolveGroup(TaintKindGroup::GROUP_INPUT),
);

$this->taint_flow_graph->addSink($sink);
Expand Down
7 changes: 7 additions & 0 deletions src/Psalm/Config.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
use Psalm\Issue\FunctionIssue;
use Psalm\Issue\MethodIssue;
use Psalm\Issue\PropertyIssue;
use Psalm\Issue\TaintTypeRegistry;
use Psalm\Issue\VariableIssue;
use Psalm\Plugin\PluginEntryPointInterface;
use Psalm\Plugin\PluginFileExtensionsInterface;
Expand Down Expand Up @@ -727,6 +728,11 @@ class Config
/** @var list<string> */
public array $config_warnings = [];

/**
* @readonly
*/
public TaintTypeRegistry $taint_type_registry;

/** @internal */
protected function __construct()
{
Expand All @@ -735,6 +741,7 @@ protected function __construct()
$this->universal_object_crates = [
strtolower(stdClass::class),
];
$this->taint_type_registry = new TaintTypeRegistry();
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/Psalm/Config/IssueHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,9 @@ public static function getAllIssueTypes(): array
&& $issue_name !== 'ParseError'
&& $issue_name !== 'PluginIssue'
&& $issue_name !== 'MixedIssue'
&& $issue_name !== 'MixedIssueTrait',
&& $issue_name !== 'MixedIssueTrait'
&& $issue_name !== 'TaintTypeFactory'
&& $issue_name !== 'TaintTypeRegistry',
);
}
}
3 changes: 2 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -516,13 +516,14 @@ private static function taintVariable(
|| $var_name === '$_REQUEST'
) {
$taint_location = new CodeLocation($statements_analyzer->getSource(), $stmt);
$taint_type_registry = Config::getInstance()->taint_type_registry;

$server_taint_source = new TaintSource(
$var_name . ':' . $taint_location->file_name . ':' . $taint_location->raw_file_start,
$var_name,
null,
null,
TaintKindGroup::ALL_INPUT,
$taint_type_registry->resolveGroup(TaintKindGroup::GROUP_INPUT),
);

$statements_analyzer->data_flow_graph->addSource($server_taint_source);
Expand Down
172 changes: 10 additions & 162 deletions src/Psalm/Internal/Codebase/TaintFlowGraph.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,8 @@
use Psalm\Internal\DataFlow\DataFlowNode;
use Psalm\Internal\DataFlow\TaintSink;
use Psalm\Internal\DataFlow\TaintSource;
use Psalm\Issue\TaintedCallable;
use Psalm\Issue\TaintedCookie;
use Psalm\Issue\TaintedCustom;
use Psalm\Issue\TaintedEval;
use Psalm\Issue\TaintedFile;
use Psalm\Issue\TaintedHeader;
use Psalm\Issue\TaintedHtml;
use Psalm\Issue\TaintedInclude;
use Psalm\Issue\TaintedLdap;
use Psalm\Issue\TaintedSSRF;
use Psalm\Issue\TaintedShell;
use Psalm\Issue\TaintedSql;
use Psalm\Issue\TaintedSystemSecret;
use Psalm\Issue\TaintedTextWithQuotes;
use Psalm\Issue\TaintedUnserialize;
use Psalm\Issue\TaintedUserSecret;
use Psalm\Issue\TaintTypeFactory;
use Psalm\IssueBuffer;
use Psalm\Type\TaintKind;

use function array_diff;
use function array_filter;
Expand Down Expand Up @@ -298,6 +282,7 @@ private function getChildNodes(

if (isset($sinks[$to_id])) {
$matching_taints = array_intersect($sinks[$to_id]->taints, $new_taints);
$matching_taints = array_filter($matching_taints);

if ($matching_taints && $generated_source->code_location) {
if ($sinks[$to_id]->code_location
Expand All @@ -312,152 +297,15 @@ private function getChildNodes(
$path = $this->getPredecessorPath($generated_source)
. ' -> ' . $this->getSuccessorPath($sinks[$to_id]);

foreach ($matching_taints as $matching_taint) {
switch ($matching_taint) {
case TaintKind::INPUT_CALLABLE:
$issue = new TaintedCallable(
'Detected tainted text',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::INPUT_UNSERIALIZE:
$issue = new TaintedUnserialize(
'Detected tainted code passed to unserialize or similar',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::INPUT_INCLUDE:
$issue = new TaintedInclude(
'Detected tainted code passed to include or similar',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::INPUT_EVAL:
$issue = new TaintedEval(
'Detected tainted code passed to eval or similar',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::INPUT_SQL:
$issue = new TaintedSql(
'Detected tainted SQL',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::INPUT_HTML:
$issue = new TaintedHtml(
'Detected tainted HTML',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::INPUT_HAS_QUOTES:
$issue = new TaintedTextWithQuotes(
'Detected tainted text with possible quotes',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::INPUT_SHELL:
$issue = new TaintedShell(
'Detected tainted shell code',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::USER_SECRET:
$issue = new TaintedUserSecret(
'Detected tainted user secret leaking',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::SYSTEM_SECRET:
$issue = new TaintedSystemSecret(
'Detected tainted system secret leaking',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::INPUT_SSRF:
$issue = new TaintedSSRF(
'Detected tainted network request',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::INPUT_LDAP:
$issue = new TaintedLdap(
'Detected tainted LDAP request',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::INPUT_COOKIE:
$issue = new TaintedCookie(
'Detected tainted cookie',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::INPUT_FILE:
$issue = new TaintedFile(
'Detected tainted file handling',
$issue_location,
$issue_trace,
$path,
);
break;

case TaintKind::INPUT_HEADER:
$issue = new TaintedHeader(
'Detected tainted header',
$issue_location,
$issue_trace,
$path,
);
break;

default:
$issue = new TaintedCustom(
'Detected tainted ' . $matching_taint,
$issue_location,
$issue_trace,
$path,
);
}
$taint_type_factory = new TaintTypeFactory($config->taint_type_registry);

foreach ($matching_taints as $matching_taint) {
$issue = $taint_type_factory->create(
$matching_taint,
$issue_location,
$issue_trace,
$path,
);
IssueBuffer::maybeAdd($issue);
}
}
Expand Down
6 changes: 2 additions & 4 deletions src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,12 +352,10 @@ public static function addDocblockInfo(
}
}

$input_taint_types = $codebase->config->taint_type_registry->resolveGroup(TaintKindGroup::GROUP_INPUT);
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,
);
$storage->taint_source_types = array_merge($storage->taint_source_types, $input_taint_types);
} else {
$storage->taint_source_types[] = $taint_source_type;
}
Expand Down
35 changes: 35 additions & 0 deletions src/Psalm/Issue/TaintTypeFactory.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

namespace Psalm\Issue;

use Psalm\CodeLocation;

final class TaintTypeFactory
{
/**
* @readonly
*/
private TaintTypeRegistry $registry;

/**
* @internal
*/
public function __construct(TaintTypeRegistry $registry)
{
$this->registry = $registry;
}

/**
* @param non-empty-string $type
*/
public function create(
string $type,
CodeLocation $code_location,
array $journey,
string $journey_text
): CodeIssue {
/** @var TaintedInput $class_name */
$class_name = $this->registry->getType($type) ?? $this->registry->getDefaultType();
return new $class_name($class_name::MESSAGE, $code_location, $journey, $journey_text);
}
}
Loading

0 comments on commit 9766b44

Please sign in to comment.