Xpath injection #10162

UPGRADING.md

@@ -11,6 +11,8 @@
 
 - [BC] The `TDependentListKey` type was removed and replaced with an optional property of the `TIntRange` type.
 
+- [BC] Value of constant `Psalm\Type\TaintKindGroup::ALL_INPUT` changed to reflect a new `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`
 
 - [BC] Methods `Codebase::getSymbolLocation()` and `Codebase::getSymbolInformation()` were replaced with `Codebase::getSymbolLocationByReference()`

config.xsd

@@ -444,6 +444,7 @@
             <xs:element name="TaintedTextWithQuotes" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedUnserialize" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedUserSecret" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="TaintedXpath" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TooFewArguments" type="ArgumentIssueHandlerType" minOccurs="0" />
             <xs:element name="TooManyArguments" type="ArgumentIssueHandlerType" minOccurs="0" />
             <xs:element name="TooManyTemplateParams" type="FunctionIssueHandlerType" minOccurs="0" />

docs/running_psalm/error_levels.md

@@ -297,6 +297,7 @@ Level 5 and above allows a more non-verifiable code, and higher levels are even
  - [TaintedSystemSecret](issues/TaintedSystemSecret.md)
  - [TaintedUnserialize](issues/TaintedUnserialize.md)
  - [TaintedUserSecret](issues/TaintedUserSecret.md)
+ - [TaintedXpath](issues/TaintedXpath.md)
  - [UncaughtThrowInGlobalScope](issues/UncaughtThrowInGlobalScope.md)
  - [UnevaluatedCode](issues/UnevaluatedCode.md)
  - [UnnecessaryVarAnnotation](issues/UnnecessaryVarAnnotation.md)

docs/running_psalm/issues.md

@@ -246,6 +246,7 @@
  - [TaintedTextWithQuotes](issues/TaintedTextWithQuotes.md)
  - [TaintedUnserialize](issues/TaintedUnserialize.md)
  - [TaintedUserSecret](issues/TaintedUserSecret.md)
+ - [TaintedXpath](issues/TaintedXpath.md)
  - [TooFewArguments](issues/TooFewArguments.md)
  - [TooManyArguments](issues/TooManyArguments.md)
  - [TooManyTemplateParams](issues/TooManyTemplateParams.md)

docs/running_psalm/issues/TaintedXpath.md

@@ -0,0 +1,12 @@
+# TaintedXpath
+
+Emitted when user-controlled input can be passed into to a xpath query.
+
+```php
+<?php
+
+function queryExpression(SimpleXMLElement $xml) : array|false|null {
+    $expression = $_GET["expression"];
+    return $xml->xpath($expression);
+}
+```

src/Psalm/Internal/Codebase/TaintFlowGraph.php

@@ -24,6 +24,7 @@
 use Psalm\Issue\TaintedTextWithQuotes;
 use Psalm\Issue\TaintedUnserialize;
 use Psalm\Issue\TaintedUserSecret;
+use Psalm\Issue\TaintedXpath;
 use Psalm\IssueBuffer;
 use Psalm\Type\TaintKind;
 
@@ -449,6 +450,15 @@ private function getChildNodes(
                                 );
                                 break;
 
+                            case TaintKind::INPUT_XPATH:
+                                $issue = new TaintedXpath(
+                                    'Detected tainted xpath query',
+                                    $issue_location,
+                                    $issue_trace,
+                                    $path,
+                                );
+                                break;
+
                             default:
                                 $issue = new TaintedCustom(
                                     'Detected tainted ' . $matching_taint,

src/Psalm/Issue/TaintedXpath.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Psalm\Issue;
+
+final class TaintedXpath extends TaintedInput
+{
+    public const SHORTCODE = 322;
+}

src/Psalm/Type/TaintKind.php

@@ -20,6 +20,7 @@ final class TaintKind
     public const INPUT_FILE = 'file';
     public const INPUT_COOKIE = 'cookie';
     public const INPUT_HEADER = 'header';
+    public const INPUT_XPATH = 'xpath';
     public const USER_SECRET = 'user_secret';
     public const SYSTEM_SECRET = 'system_secret';
 }

src/Psalm/Type/TaintKindGroup.php

@@ -7,7 +7,7 @@
  */
 final class TaintKindGroup
 {
     public const ALL_INPUT = [
         TaintKind::INPUT_HTML,
         TaintKind::INPUT_HAS_QUOTES,
         TaintKind::INPUT_SHELL,
@@ -21,5 +21,6 @@
         TaintKind::INPUT_FILE,
         TaintKind::INPUT_HEADER,
         TaintKind::INPUT_COOKIE,
+        TaintKind::INPUT_XPATH,
     ];
 }

stubs/extensions/dom.phpstub

@@ -972,10 +972,15 @@ class DOMXPath
 
     public function __construct(DOMDocument $document, bool $registerNodeNS = true) {}
 
+    /**
+     * @return DOMNodeList<DOMNode>|false
+     * @psalm-taint-sink xpath $expression
+     */
     public function evaluate(string $expression, ?DOMNode $contextNode = null, bool $registerNodeNS = true): mixed {}
 
     /**
      * @return DOMNodeList<DOMNode>|false
+     * @psalm-taint-sink xpath $expression
      */
     public function query(string $expression, ?DOMNode $contextNode = null, bool $registerNodeNS = true): mixed {}
 

stubs/extensions/simplexml.phpstub

@@ -29,7 +29,10 @@ function simplexml_import_dom(SimpleXMLElement|DOMNode $node, ?string $class_nam
  */
 class SimpleXMLElement implements Traversable, Countable
 {
-    /** @return array<array-key, SimpleXMLElement>|null|false */
+    /**
+     * @return array<array-key, SimpleXMLElement>|null|false
+     * @psalm-taint-sink xpath $expression
+     */
     public function xpath(string $expression) {}
 
     public function registerXPathNamespace(string $prefix, string $namespace): bool {}

tests/TaintTest.php

@@ -750,6 +750,19 @@ function bar(array $arr): void {
 
                     $mysqli->query("$a$b$c$d");',
             ],
+            'querySimpleXMLElement' => [
+                'code' => '<?php
+                    /**
+                     * @psalm-taint-escape xpath
+                     */
+                    function my_escaping_function_for_xpath(string input) : string {};
+
+                    function queryExpression(SimpleXMLElement $xml) : array|false|null {
+                        $expression = $_GET["expression"];
+                        $expression = my_escaping_function_for_xpath($expression);
+                        return $xml->xpath($expression);
+                    }',
+            ],
         ];
     }
 
@@ -2503,6 +2516,30 @@ public static function getPrevious(string $s): string {
                     $function->invoke();',
                 'error_message' => 'TaintedCallable',
             ],
+            'querySimpleXMLElement' => [
+                'code' => '<?php
+                    function queryExpression(SimpleXMLElement $xml) : array|false|null {
+                        $expression = $_GET["expression"];
+                        return $xml->xpath($expression);
+                    }',
+                'error_message' => 'TaintedXpath',
+            ],
+            'queryDOMXPath' => [
+                'code' => '<?php
+                    function queryExpression(DOMXPath $xpath) : mixed {
+                        $expression = $_GET["expression"];
+                        return $xpath->query($expression);
+                    }',
+                'error_message' => 'TaintedXpath',
+            ],
+            'evaluateDOMXPath' => [
+                'code' => '<?php
+                    function evaluateExpression(DOMXPath $xpath) : mixed {
+                        $expression = $_GET["expression"];
+                        return $xpath->evaluate($expression);
+                    }',
+                'error_message' => 'TaintedXpath',
+            ],
         ];
     }