Skip to content

Commit

Permalink
Implement TaintedSession
Browse files Browse the repository at this point in the history
  • Loading branch information
cgocast committed Jun 4, 2024
1 parent 16b24bd commit 1363c24
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 1 deletion.
2 changes: 1 addition & 1 deletion UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

- [BC] Class `Psalm\Issue\MixedInferredReturnType` was removed

- [BC] Value of constant `Psalm\Type\TaintKindGroup::ALL_INPUT` changed to reflect new `TaintKind::INPUT_EXTRACT`, `TaintKind::INPUT_SLEEP` and `TaintKind::INPUT_XPATH` have been added. Accordingly, default values for `$taint` parameters of `Psalm\Codebase::addTaintSource()` and `Psalm\Codebase::addTaintSink()` have been changed as well.
- [BC] Value of constant `Psalm\Type\TaintKindGroup::ALL_INPUT` changed to reflect new `TaintKind::INPUT_EXTRACT`, `TaintKind::INPUT_SESSION`, `TaintKind::INPUT_SLEEP` and `TaintKind::INPUT_XPATH` have been added. Accordingly, default values for `$taint` parameters of `Psalm\Codebase::addTaintSource()` and `Psalm\Codebase::addTaintSink()` have been changed as well.

- [BC] Property `Config::$shepherd_host` was replaced with `Config::$shepherd_endpoint`

Expand Down
1 change: 1 addition & 0 deletions config.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@
<xs:element name="TaintedInclude" type="IssueHandlerType" minOccurs="0" />
<xs:element name="TaintedInput" type="IssueHandlerType" minOccurs="0" />
<xs:element name="TaintedLdap" type="IssueHandlerType" minOccurs="0" />
<xs:element name="TaintedSession" type="IssueHandlerType" minOccurs="0" />
<xs:element name="TaintedShell" type="IssueHandlerType" minOccurs="0" />
<xs:element name="TaintedSleep" type="IssueHandlerType" minOccurs="0" />
<xs:element name="TaintedSql" type="IssueHandlerType" minOccurs="0" />
Expand Down
1 change: 1 addition & 0 deletions docs/running_psalm/error_levels.md
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ Level 5 and above allows a more non-verifiable code, and higher levels are even
- [TaintedInclude](issues/TaintedInclude.md)
- [TaintedInput](issues/TaintedInput.md)
- [TaintedLdap](issues/TaintedLdap.md)
- [TaintedSession](issues/TaintedSession.md)
- [TaintedShell](issues/TaintedShell.md)
- [TaintedSleep](issues/TaintedSleep.md)
- [TaintedSql](issues/TaintedSql.md)
Expand Down
1 change: 1 addition & 0 deletions docs/running_psalm/issues.md
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@
- [TaintedInclude](issues/TaintedInclude.md)
- [TaintedInput](issues/TaintedInput.md)
- [TaintedLdap](issues/TaintedLdap.md)
- [TaintedSession](issues/TaintedSession.md)
- [TaintedShell](issues/TaintedShell.md)
- [TaintedSleep](issues/TaintedSleep.md)
- [TaintedSql](issues/TaintedSql.md)
Expand Down
18 changes: 18 additions & 0 deletions docs/running_psalm/issues/TaintedSession.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# TaintedExtract

Emitted when user-controlled input can be passed into the `$_SESSION` array.

## Example

```php
<?php

$usrname = $_GET["usrname"];
if (!isset($_SESSION["attr_user"])) {
$_SESSION["attr_user"] = $usrname;
}
```

## Further ressource

[CWE-501: Trust Boundary Violation](https://cwe.mitre.org/data/definitions/501.html)
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Psalm\Internal\Codebase\TaintFlowGraph;
use Psalm\Internal\DataFlow\DataFlowNode;
use Psalm\Internal\DataFlow\TaintSource;
use Psalm\Internal\DataFlow\TaintSink;
use Psalm\Issue\ImpureVariable;
use Psalm\Issue\InvalidScope;
use Psalm\Issue\PossiblyUndefinedGlobalVariable;
Expand All @@ -33,6 +34,7 @@
use Psalm\Type\Atomic\TNonEmptyString;
use Psalm\Type\Atomic\TNull;
use Psalm\Type\Atomic\TString;
use Psalm\Type\TaintKind;
use Psalm\Type\TaintKindGroup;
use Psalm\Type\Union;

Expand Down Expand Up @@ -533,6 +535,19 @@ private static function taintVariable(
$server_taint_source->id => $server_taint_source,
]);
}
if ($var_name === '$_SESSION') {
$sink_location = new CodeLocation($statements_analyzer->getSource(), $stmt);

$echo_param_sink = TaintSink::getForAssignment($var_name, $sink_location);

$echo_param_sink->taints = [
TaintKind::INPUT_SESSION,
TaintKind::USER_SECRET,
TaintKind::SYSTEM_SECRET,
];

$statements_analyzer->data_flow_graph->addSink($echo_param_sink);
}
}
}

Expand Down
10 changes: 10 additions & 0 deletions src/Psalm/Internal/Codebase/TaintFlowGraph.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use Psalm\Issue\TaintedInclude;
use Psalm\Issue\TaintedLdap;
use Psalm\Issue\TaintedSSRF;
use Psalm\Issue\TaintedSession;
use Psalm\Issue\TaintedShell;
use Psalm\Issue\TaintedSleep;
use Psalm\Issue\TaintedSql;
Expand Down Expand Up @@ -478,6 +479,15 @@ private function getChildNodes(
);
break;

case TaintKind::INPUT_SESSION:
$issue = new TaintedSession(
'Detected tainted session',
$issue_location,
$issue_trace,
$path,
);
break;

default:
$issue = new TaintedCustom(
'Detected tainted ' . $matching_taint,
Expand Down
10 changes: 10 additions & 0 deletions src/Psalm/Issue/TaintedSession.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

namespace Psalm\Issue;

final class TaintedSession extends TaintedInput
{
public const SHORTCODE = 329;
}
1 change: 1 addition & 0 deletions src/Psalm/Type/TaintKind.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ final class TaintKind
public const INPUT_XPATH = 'xpath';
public const INPUT_SLEEP = 'sleep';
public const INPUT_EXTRACT = 'extract';
public const INPUT_SESSION = 'session';
public const USER_SECRET = 'user_secret';
public const SYSTEM_SECRET = 'system_secret';
}
1 change: 1 addition & 0 deletions src/Psalm/Type/TaintKindGroup.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ final class TaintKindGroup
TaintKind::INPUT_XPATH,
TaintKind::INPUT_SLEEP,
TaintKind::INPUT_EXTRACT,
TaintKind::INPUT_SESSION,
];
}
5 changes: 5 additions & 0 deletions tests/TaintTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2596,6 +2596,11 @@ function evaluateExpression(DOMXPath $xpath) : mixed {
extract($_POST);',
'error_message' => 'TaintedExtract',
],
'taintedSession' => [
'code' => '<?php
$_SESSION["foo"] = $_GET["foo"];',
'error_message' => 'TaintedSession',
],
];
}

Expand Down

0 comments on commit 1363c24

Please sign in to comment.