Add SSRF sinks (#4592)

config.xsd

@@ -379,6 +379,7 @@
             <xs:element name="TaintedInput" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedShell" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedSql" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="TaintedSSRF" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedSystemSecret" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedText" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedUnserialize" type="IssueHandlerType" minOccurs="0" />

dictionaries/InternalTaintSinkMap.php

@@ -41,4 +41,6 @@ return [
 'unserialize' => [['unserialize']],
 'popen' => [['shell']],
 'proc_open' => [['shell']],
+'curl_init' => [['ssrf']],
+'curl_setopt' => [[], [], ['ssrf']],
 ];

docs/running_psalm/issues/TaintedSSRF.md

@@ -0,0 +1,36 @@
+# TaintedSSRF
+
+Potential Server-Side Request Forgery vulnerability. This rule is emitted when user-controlled input can be passed into a network request.
+
+## Risk
+
+Passing untrusted user input to network requests could be dangerous. 
+
+If an attacker can fully control a HTTP request they could connect to internal services. Depending on the nature of these, this can pose a security risk. (e.g. backend services, admin interfaces, AWS metadata, ...)
+
+## Example
+
+```php
+<?php
+$ch = curl_init();
+
+curl_setopt($ch, CURLOPT_URL, $_GET['url']);
+
+curl_exec($ch);
+
+curl_close($ch);
+```
+
+## Mitigations
+
+Mitigating SSRF vulnerabilities can be tricky. Disallowing IPs would likely not work as an attacker could create a malicious domain that points to an internal DNS name.
+
+Consider:
+
+1. Having an allow list of domains that can be connected to.
+2. Pointing cURL to a proxy that has no access to internal resources.
+
+## Further resources
+
+- [OWASP Wiki for Server Side Request Forgery](https://owasp.org/www-community/attacks/Server_Side_Request_Forgery)
+- [CWE-918](https://cwe.mitre.org/data/definitions/918)

src/Psalm/Internal/Codebase/TaintFlowGraph.php

@@ -11,6 +11,7 @@ use Psalm\Issue\TaintedEval;
 use Psalm\Issue\TaintedHtml;
 use Psalm\Issue\TaintedInclude;
 use Psalm\Issue\TaintedShell;
+use Psalm\Issue\TaintedSSRF;
 use Psalm\Issue\TaintedSql;
 use Psalm\Issue\TaintedSystemSecret;
 use Psalm\Issue\TaintedText;
@@ -355,6 +356,15 @@ class TaintFlowGraph extends DataFlowGraph
                                 );
                                 break;
 
+                            case TaintKind::INPUT_SSRF:
+                                $issue = new TaintedSSRF(
+                                    'Detected tainted network request',
+                                    $issue_location,
+                                    $issue_trace,
+                                    $path
+                                );
+                                break;
+                                
                             default:
                                 $issue = new TaintedCustom(
                                     'Detected tainted ' . $matching_taint,

src/Psalm/Issue/TaintedSSRF.php

@@ -0,0 +1,7 @@
+<?php
+namespace Psalm\Issue;
+
+class TaintedSSRF extends TaintedInput
+{
+    public const SHORTCODE = 253;
+}

src/Psalm/Type/TaintKind.php

@@ -14,6 +14,7 @@ class TaintKind
     public const INPUT_SQL = 'sql';
     public const INPUT_HTML = 'html';
     public const INPUT_SHELL = 'shell';
+    public const INPUT_SSRF = 'ssrf';
     public const USER_SECRET = 'user_secret';
     public const SYSTEM_SECRET = 'system_secret';
 }

src/Psalm/Type/TaintKindGroup.php

@@ -15,5 +15,6 @@ class TaintKindGroup
         TaintKind::INPUT_EVAL,
         TaintKind::INPUT_UNSERIALIZE,
         TaintKind::INPUT_INCLUDE,
+        TaintKind::INPUT_SSRF,
     ];
 }

tests/TaintTest.php

@@ -1640,6 +1640,17 @@ class TaintTest extends TestCase
                     $cb = proc_open($_POST[\'x\'], [], []);',
                 'error_message' => 'TaintedShell',
             ],
+            'taintedCurlInit' => [
+                '<?php
+                    $ch = curl_init($_GET[\'url\']);',
+                'error_message' => 'TaintedSSRF',
+            ],
+            'taintedCurlSetOpt' => [
+                '<?php
+                    $ch = curl_init();
+                    curl_setopt($ch, CURLOPT_URL, $_GET[\'url\']);',
+                'error_message' => 'TaintedSSRF',
+            ],
             /*
             // TODO: Stubs do not support this type of inference even with $this->message = $message.
             // Most uses of getMessage() would be with caught exceptions, so this is not representative of real code.