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::get('html'); // returns \Psalm\Issue\TaintedHtml
TaintTypeRegistry::add('my-type', \Example\TaintedMyType::class);
```
  • Loading branch information
@ohader
ohader committed Feb 22, 2024
1 parent 4a5f433 commit 402317b
Show file tree
Hide file tree
Showing 24 changed files with 224 additions and 163 deletions.
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',
);
}
}
171 changes: 9 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 @@ -312,152 +296,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
29 changes: 29 additions & 0 deletions src/Psalm/Issue/TaintTypeFactory.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

namespace Psalm\Issue;

use Psalm\CodeLocation;

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

public function __construct(TaintTypeRegistry $registry)
{
$this->registry = $registry;
}

public function create(
string $type,
CodeLocation $code_location,
array $journey,
string $journey_text
): CodeIssue {
/** @var TaintedInput $class_name */
$class_name = $this->registry->get($type) ?? $this->registry->getDefault();
return new $class_name($class_name::MESSAGE, $code_location, $journey, $journey_text);
}
}
93 changes: 93 additions & 0 deletions src/Psalm/Issue/TaintTypeRegistry.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
<?php

namespace Psalm\Issue;

use LogicException;
use Psalm\Type\TaintKind;
use ReflectionClass;
use RuntimeException;

use function class_exists;
use function stripos;

final class TaintTypeRegistry
{
/**
* @var array<string, class-string<CodeIssue>>
*/
private array $taint_types = [
TaintKind::INPUT_CALLABLE => TaintedCallable::class,
TaintKind::INPUT_UNSERIALIZE => TaintedUnserialize::class,
TaintKind::INPUT_INCLUDE => TaintedInclude::class,
TaintKind::INPUT_EVAL => TaintedEval::class,
TaintKind::INPUT_SQL => TaintedSql::class,
TaintKind::INPUT_HTML => TaintedHtml::class,
TaintKind::INPUT_HAS_QUOTES => TaintedTextWithQuotes::class,
TaintKind::INPUT_SHELL => TaintedShell::class,
TaintKind::USER_SECRET => TaintedUserSecret::class,
TaintKind::SYSTEM_SECRET => TaintedSystemSecret::class,
TaintKind::INPUT_SSRF => TaintedSSRF::class,
TaintKind::INPUT_LDAP => TaintedLdap::class,
TaintKind::INPUT_COOKIE => TaintedCookie::class,
TaintKind::INPUT_FILE => TaintedFile::class,
TaintKind::INPUT_HEADER => TaintedHeader::class,
];

/**
* @var class-string<CodeIssue>
*/
private string $default = TaintedCustom::class;

/**
* @param class-string<CodeIssue> $class_name
*/
public function add(string $type, string $class_name): void
{
if ($this->has($type)) {
throw new RuntimeException('Taint type ' . $type . ' is already defined');
}
$this->assertClassName($class_name);
$this->taint_types[$type] = $class_name;
}

public function has(string $type): bool
{
return isset($this->taint_types[$type]);
}

/**
* @return class-string<CodeIssue>|null
*/
public function get(string $type): ?string
{
return $this->taint_types[$type] ?? null;
}

/**
* @return class-string<CodeIssue>
*/
public function getDefault(): string
{
return $this->default;
}

/**
* @psalm-suppress PossiblyUnusedMethod
* @param class-string<CodeIssue> $class_name
*/
public function setDefault(string $class_name): void
{
$this->assertClassName($class_name);
$this->default = $class_name;
}

private function assertClassName(string $class_name): void
{
if (!class_exists($class_name)) {
throw new LogicException('Taint class ' . $class_name . ' does not exist');
}
if (stripos((new ReflectionClass($class_name))->getShortName(), 'Tainted') !== 0) {
throw new LogicException('Taint class name ' . $class_name . ' must start with "Tainted"');
}
}
}
1 change: 1 addition & 0 deletions src/Psalm/Issue/TaintedCallable.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
final class TaintedCallable extends TaintedInput
{
public const SHORTCODE = 243;
public const MESSAGE = 'Detected tainted text';
}
1 change: 1 addition & 0 deletions src/Psalm/Issue/TaintedCookie.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
final class TaintedCookie extends TaintedInput
{
public const SHORTCODE = 257;
public const MESSAGE = 'Detected tainted cookie';
}
1 change: 1 addition & 0 deletions src/Psalm/Issue/TaintedCustom.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
final class TaintedCustom extends TaintedInput
{
public const SHORTCODE = 249;
public const MESSAGE = 'Detected tainted %s';
}
1 change: 1 addition & 0 deletions src/Psalm/Issue/TaintedEval.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
final class TaintedEval extends TaintedInput
{
public const SHORTCODE = 252;
public const MESSAGE = 'Detected tainted code passed to eval or similar';
}
1 change: 1 addition & 0 deletions src/Psalm/Issue/TaintedFile.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
final class TaintedFile extends TaintedInput
{
public const SHORTCODE = 255;
public const MESSAGE = 'Detected tainted file handling';
}
1 change: 1 addition & 0 deletions src/Psalm/Issue/TaintedHeader.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
final class TaintedHeader extends TaintedInput
{
public const SHORTCODE = 256;
public const MESSAGE = 'Detected tainted header';
}
1 change: 1 addition & 0 deletions src/Psalm/Issue/TaintedHtml.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
final class TaintedHtml extends TaintedInput
{
public const SHORTCODE = 245;
public const MESSAGE = 'Detected tainted HTML';
}
1 change: 1 addition & 0 deletions src/Psalm/Issue/TaintedInclude.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
final class TaintedInclude extends TaintedInput
{
public const SHORTCODE = 251;
public const MESSAGE = 'Detected tainted code passed to include or similar';
}
Loading

0 comments on commit 402317b

Please sign in to comment.