diff --git a/src/Psalm/Internal/Codebase/TaintFlowGraph.php b/src/Psalm/Internal/Codebase/TaintFlowGraph.php index c0a6162d781..c68bce4a89d 100644 --- a/src/Psalm/Internal/Codebase/TaintFlowGraph.php +++ b/src/Psalm/Internal/Codebase/TaintFlowGraph.php @@ -1213,7 +1213,9 @@ private function getChildNodes( continue; } - if ($sink !== null && $generated_source->code_location) { + // a flow is reported at its sink, or else at the node it reaches the sink from: a plugin can + // connect a node without a location to a sink + if ($sink !== null && ($generated_source->code_location || $sink->code_location)) { $matching_taints = $sink->taints & $new_taints; if ($matching_taints) { @@ -1332,7 +1334,13 @@ private function reportTaintedFlowOnce( Config $config, Codebase $codebase, ): void { - if ($predecessor->code_location === null) { + if ($sink->code_location + && $config->reportIssueInFile('TaintedInput', $sink->code_location->file_path) + ) { + $issue_location = $sink->code_location; + } elseif ($predecessor->code_location !== null) { + $issue_location = $predecessor->code_location; + } else { return; } @@ -1351,14 +1359,6 @@ private function reportTaintedFlowOnce( $this->reported_flows[$sink->id][$predecessor->id][$origin] = $reported_taints | $unreported_taints; - if ($sink->code_location - && $config->reportIssueInFile('TaintedInput', $sink->code_location->file_path) - ) { - $issue_location = $sink->code_location; - } else { - $issue_location = $predecessor->code_location; - } - $issue_trace = $this->getIssueTrace($predecessor); $path = $this->getPredecessorPath($predecessor) . ' -> ' . $this->getSuccessorPath($sink); diff --git a/tests/Config/Plugin/EventHandler/LocationlessNode/LocationlessNodeTest.php b/tests/Config/Plugin/EventHandler/LocationlessNode/LocationlessNodeTest.php new file mode 100644 index 00000000000..8144c5249cd --- /dev/null +++ b/tests/Config/Plugin/EventHandler/LocationlessNode/LocationlessNodeTest.php @@ -0,0 +1,100 @@ +setIncludeCollector(new IncludeCollector()); + $p = new ProjectAnalyzer( + $config, + new Providers( + $this->file_provider, + new FakeParserCacheProvider(), + ), + new ReportOptions(), + ); + $p->initExtraFiles(); + $p->initProjectFiles(); + return $p; + } + + public function testFlowFromANodeWithoutLocationIsReportedAtTheSink(): void + { + $this->project_analyzer = $this->getProjectAnalyzerWithConfig( + TestConfig::loadFromXML( + dirname(__DIR__, 5) . DIRECTORY_SEPARATOR, + ' + + + + + + + + + ', + ), + ); + $this->project_analyzer->getCodebase()->config->eventDispatcher->registerClass(RelayPlugin::class); + + $file_path = (string) getcwd() . '/src/somefile.php'; + + $this->addFile( + $file_path, + 'expectException(CodeException::class); + $this->expectExceptionMessageMatches('#^TaintedHtml - (.*[\\\\/])?src[\\\\/]somefile\.php:7:13#'); + + $this->analyzeFile($file_path, new Context(), true, true); + } +} diff --git a/tests/Config/Plugin/EventHandler/LocationlessNode/RelayPlugin.php b/tests/Config/Plugin/EventHandler/LocationlessNode/RelayPlugin.php new file mode 100644 index 00000000000..5acd01a1e3b --- /dev/null +++ b/tests/Config/Plugin/EventHandler/LocationlessNode/RelayPlugin.php @@ -0,0 +1,53 @@ +getExpr(); + $graph = $event->getCodebase()->taint_flow_graph; + + if ($graph === null || !$expr instanceof FuncCall || !$expr->name instanceof Name) { + return null; + } + + $relay = DataFlowNode::getForPropertyFetch('relay'); + $graph->addNode($relay); + + if ($expr->name->toLowerString() === 'relay' && isset($expr->getArgs()[0])) { + $type = $event->getStatementsSource()->getNodeTypeProvider()->getType($expr->getArgs()[0]->value); + foreach ($type?->parent_nodes ?? [] as $parent_node) { + $graph->addPath($parent_node, $relay, 'arg'); + } + } elseif ($expr->name->toLowerString() === 'deliver') { + $sink = DataFlowNode::getForTaint( + 'deliver', + new CodeLocation($event->getStatementsSource(), $expr), + TaintKind::INPUT_HTML, + ); + $graph->addNode($sink); + $graph->addSink($sink); + $graph->addPath($relay, $sink, 'arg'); + } + + return null; + } +}