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

```
$registry = Config::getInstance()->taint_kind_registry;

$registry->defineKinds([
  'mine' => \Example\Package\TaintedMine::class,
  'yours' => \Example\Package\TaintedYours::class,
], TaintKindGroup::GROUP_INPUT);

$registry->defineGroup('my-input', 'html', 'sql', 'shell');

$registry->defineGroupProxy('input-sql', 'input', [
  'sql' => \Example\Package\TaintedSqlSecondOrder::class,
]);

$registry->getKind('html'); // returns TaintedHtml::class;
$registry->getGroupKinds('input'); // returns ['html', 'sql', ...]
```
  • Loading branch information
ohader committed Feb 26, 2024
1 parent 71707e6 commit c558108
Show file tree
Hide file tree
Showing 31 changed files with 781 additions and 180 deletions.
99 changes: 99 additions & 0 deletions docs/security_analysis/custom_taint_kinds.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# Custom Taint Kinds

[Psalm\Type\TaintKindRegistry](https://github.com/vimeo/psalm/blob/master/src/Psalm/Type/TaintKindRegistry.php)
allows plugins to define their own taint kinds and groups. The registry can be accessed via the `Psalm\Config`
singleton.


## Define Kinds

```php
$registry = \Psalm\Config::getInstance()->taint_kind_registry;
$registry->defineKinds([
'custom-first' => \Example\Package\TaintedCustomA::class,
'custom-second' => \Example\Package\TaintedCustomB::class,
], 'custom');
```

The example above defines the custom taints `custom-first` and `custom-second`,
maps them to corresponding individual issue classes and organizes them
in a new custom group `custom`.

```php
/**
* @psalm-taint-source custom-first
*/
function fetchFirst(): string {}
/**
* Stub for the `custom` group - which includes both defined kinds.
*
* @psalm-taint-source custom
*/
function fetchData(): array {}
```

## Define Groups

```php
$registry = \Psalm\Config::getInstance()->taint_kind_registry;
$registry->defineGroup('specific-input', 'html', 'sql');
```

The exmaple above defines a new group `specific-input`, which only holds
a reduced subset (`html` and `sql`) of the default built-in group `input`.

```php
/**
* @psalm-taint-source specific-input
*/
function fetchData(): array {}
```

## Extend Groups

```php
$registry = \Psalm\Config::getInstance()->taint_kind_registry;
$registry->extendGroup('input', 'custom-first');
```

The example above adds the custom kind `custom-first` to an existing group `input`.

## Define Group Proxies

```php
$registry = \Psalm\Config::getInstance()->taint_kind_registry;
$registry->defineGroupProxy('input-sql', 'input', [
'sql' => \Example\Package\TaintedSqlSecondOrder::class,
]);
```

The example above is special as it defines `input-sql` to be a proxy for
the existing group `input-sql` and keeps track of that usage. In addition,
the build-in kind `sql` is overloaded with a custom issue class.

This example is known as "Second Order SQL Injection", where data that was
retrieved from a database is reused for new SQL queries. The assumption is,
that the persisted data was not sanitized and might contain user submitted
malicious snippets.

```php
/**
* @psalm-taint-source input-sql
*/
function fetchAliasFromDatabase(): string {}

/**
* @psalm-taint-sink sql $query
*/
function executeDatabaseQuery($query): array {}

// this would still issue the default `TaintSql`
$alias = $_GET['alias'];
$query = sprintf('SELECT * FROM comments WHERE alias="%s";', $alias);
$rows = executeDatabaseQuery($query);

// this would issue the custom `TaintedSqlSecondOrder`
$alias = fetchAliasFromDatabase();
$query = sprintf('SELECT * FROM comments WHERE alias="%s";', $alias);
$rows = executeDatabaseQuery($query);
```
3 changes: 2 additions & 1 deletion docs/security_analysis/custom_taint_sources.md
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_kind_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,
[TaintKindGroup::GROUP_INPUT],
new CodeLocation($statements_source, $expr)
);
}
Expand Down
7 changes: 7 additions & 0 deletions docs/security_analysis/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,14 @@ Psalm recognises a number of taint types by default, defined in the [Psalm\Type\
- `user_secret` - used for strings that could contain user-supplied secrets
- `system_secret` - used for strings that could contain system secrets

Each of these default kind types will be mapped to a corresponding class which inherits from abstract class
`TaintedInput` - for instance `TaintedHtml`, `TaintedShell`, `TaintedHeader`, etc.

You're also free to define your own taint types when defining custom taint sources – they're just strings.
Per default those custom types are mapped to the generic class `TaintedCustom`.

However, plugins can utilize [custom taint kinds](custom_taint_kinds.md) to define more specific classes
for improved semantics and enhanced issue handling in general.

## Taint Sources

Expand Down
4 changes: 2 additions & 2 deletions src/Psalm/Codebase.php
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 array ( 0 => 'input', )
Union $expr_type,
string $taint_id,
array $taints = TaintKindGroup::ALL_INPUT,
array $taints = [TaintKindGroup::GROUP_INPUT],
?CodeLocation $code_location = null
): Union {
if (!$this->taint_flow_graph) {
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 array ( 0 => 'input', )
string $taint_id,
array $taints = TaintKindGroup::ALL_INPUT,
array $taints = [TaintKindGroup::GROUP_INPUT],
?CodeLocation $code_location = null
): void {
if (!$this->taint_flow_graph) {
Expand Down
7 changes: 7 additions & 0 deletions src/Psalm/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
use Psalm\Plugin\PluginInterface;
use Psalm\Progress\Progress;
use Psalm\Progress\VoidProgress;
use Psalm\Type\TaintKindRegistry;
use RuntimeException;
use SimpleXMLElement;
use Symfony\Component\Filesystem\Path;
Expand Down Expand Up @@ -727,6 +728,11 @@ class Config
/** @var list<string> */
public array $config_warnings = [];

/**
* @readonly
*/
public TaintKindRegistry $taint_kind_registry;

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

/**
Expand Down
3 changes: 2 additions & 1 deletion src/Psalm/Config/IssueHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ public static function getAllIssueTypes(): array
&& $issue_name !== 'ParseError'
&& $issue_name !== 'PluginIssue'
&& $issue_name !== 'MixedIssue'
&& $issue_name !== 'MixedIssueTrait',
&& $issue_name !== 'MixedIssueTrait'
&& $issue_name !== 'TaintKindFactory',
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ private static function taintVariable(
$var_name,
null,
null,
TaintKindGroup::ALL_INPUT,
[TaintKindGroup::GROUP_INPUT],
);

$statements_analyzer->data_flow_graph->addSource($server_taint_source);
Expand Down
Loading

0 comments on commit c558108

Please sign in to comment.