Implement TaintedSession

UPGRADING.md

@@ -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`
 

config.xsd

@@ -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" />

docs/running_psalm/error_levels.md

@@ -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)

docs/running_psalm/issues.md

@@ -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)

docs/running_psalm/issues/TaintedSession.md

@@ -0,0 +1,18 @@
+# TaintedSession
+
+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)

src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php

@@ -14,6 +14,7 @@
 use Psalm\Internal\Analyzer\StatementsAnalyzer;
 use Psalm\Internal\Codebase\TaintFlowGraph;
 use Psalm\Internal\DataFlow\DataFlowNode;
+use Psalm\Internal\DataFlow\TaintSink;
 use Psalm\Internal\DataFlow\TaintSource;
 use Psalm\Issue\ImpureVariable;
 use Psalm\Issue\InvalidScope;
@@ -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;
 
@@ -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);
+            }
         }
     }
 

src/Psalm/Internal/Codebase/TaintFlowGraph.php

@@ -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;
@@ -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,

src/Psalm/Issue/TaintedSession.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Psalm\Issue;
+
+final class TaintedSession extends TaintedInput
+{
+    public const SHORTCODE = 329;
+}

src/Psalm/Type/TaintKind.php

@@ -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';
 }

src/Psalm/Type/TaintKindGroup.php

@@ -11,7 +11,7 @@
 {
     public const GROUP_INPUT = 'input';
 
     public const ALL_INPUT = [
         TaintKind::INPUT_HTML,
         TaintKind::INPUT_HAS_QUOTES,
         TaintKind::INPUT_SHELL,
@@ -28,5 +28,6 @@
         TaintKind::INPUT_XPATH,
         TaintKind::INPUT_SLEEP,
         TaintKind::INPUT_EXTRACT,
+        TaintKind::INPUT_SESSION,
     ];
 }

tests/TaintTest.php

@@ -2596,6 +2596,11 @@ function evaluateExpression(DOMXPath $xpath) : mixed {
                     extract($_POST);',
                 'error_message' => 'TaintedExtract',
             ],
+            'taintedSession' => [
+                'code' => '<?php
+                    $_SESSION["foo"] = $_GET["foo"];',
+                'error_message' => 'TaintedSession',
+            ],
         ];
     }