Skip to content

Commit 3675b84

Browse files
authored
Merge pull request #11384 from vimeo/combined_analysis
Combined analysis
2 parents 1288d7a + c6b1bfa commit 3675b84

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+1372
-1097
lines changed

docs/security_analysis/index.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
Psalm can attempt to find connections between user-controlled input (like `$_GET['name']`) and places that we don’t want unescaped user-controlled input to end up (like `echo "<h1>$name</h1>"` by looking at the ways that data flows through your application (via assignments, function/method calls and array/property access).
44

5-
You can enable this mode with the `--taint-analysis` command line flag. When taint analysis is enabled, no other analysis is performed. To [ensure comprehensive results](https://github.com/vimeo/psalm/issues/6156), Psalm should be run normally prior to taint analysis, and any errors should be fixed.
5+
You can enable this mode with the `--taint-analysis` command line flag.
66

77
Tainted input is anything that can be controlled, wholly or in part, by a user of your application. In taint analysis, tainted input is called a _taint source_.
88

psalm-baseline.xml

+48-11
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<?xml version="1.0" encoding="UTF-8"?>
2-
<files psalm-version="dev-master@354b4d49a646d88aeb9612668c15402416cc70c0">
2+
<files psalm-version="dev-master@a4b37609d5438890372257fef3012cad3e0ca48d">
33
<file src="examples/TemplateChecker.php">
44
<PossiblyUndefinedIntArrayOffset>
55
<code><![CDATA[$comment_block->tags['variablesfrom'][0]]]></code>
@@ -154,6 +154,11 @@
154154
<code><![CDATA[$creating_conditional_id]]></code>
155155
</RiskyTruthyFalsyComparison>
156156
</file>
157+
<file src="src/Psalm/Internal/Algebra/FormulaGenerator.php">
158+
<ComplexMethod>
159+
<code><![CDATA[getFormula]]></code>
160+
</ComplexMethod>
161+
</file>
157162
<file src="src/Psalm/Internal/Analyzer/AlgebraAnalyzer.php">
158163
<ImplicitToStringCast>
159164
<code><![CDATA[$clause_1]]></code>
@@ -403,6 +408,9 @@
403408
</ArgumentTypeCoercion>
404409
</file>
405410
<file src="src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php">
411+
<ComplexMethod>
412+
<code><![CDATA[checkIteratorType]]></code>
413+
</ComplexMethod>
406414
<ConflictingReferenceConstraint>
407415
<code><![CDATA[if (AtomicTypeComparator::isContainedBy(]]></code>
408416
<code><![CDATA[if (AtomicTypeComparator::isContainedBy(]]></code>
@@ -452,6 +460,11 @@
452460
<code><![CDATA[$statements_analyzer->getTemplateTypeMap()]]></code>
453461
</RiskyTruthyFalsyComparison>
454462
</file>
463+
<file src="src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php">
464+
<ComplexMethod>
465+
<code><![CDATA[analyze]]></code>
466+
</ComplexMethod>
467+
</file>
455468
<file src="src/Psalm/Internal/Analyzer/Statements/Block/SwitchAnalyzer.php">
456469
<InvalidPropertyAssignmentValue>
457470
<code><![CDATA[$context->assigned_var_ids += $switch_scope->new_assigned_var_ids]]></code>
@@ -936,16 +949,6 @@
936949
</RiskyTruthyFalsyComparison>
937950
</file>
938951
<file src="src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php">
939-
<PossiblyNullArgument>
940-
<code><![CDATA[$function_name]]></code>
941-
<code><![CDATA[$function_name]]></code>
942-
</PossiblyNullArgument>
943-
<PossiblyNullPropertyFetch>
944-
<code><![CDATA[$stmt->getArgs()[0]->value]]></code>
945-
</PossiblyNullPropertyFetch>
946-
<PossiblyUndefinedArrayOffset>
947-
<code><![CDATA[$stmt->getArgs()[0]]]></code>
948-
</PossiblyUndefinedArrayOffset>
949952
<PossiblyUndefinedIntArrayOffset>
950953
<code><![CDATA[$parts[1]]]></code>
951954
</PossiblyUndefinedIntArrayOffset>
@@ -989,6 +992,9 @@
989992
</RiskyTruthyFalsyComparison>
990993
</file>
991994
<file src="src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/ExistingAtomicMethodCallAnalyzer.php">
995+
<ComplexMethod>
996+
<code><![CDATA[analyze]]></code>
997+
</ComplexMethod>
992998
<ImplicitToStringCast>
993999
<code><![CDATA[$method_id]]></code>
9941000
<code><![CDATA[$pseudo_set_type]]></code>
@@ -1120,6 +1126,9 @@
11201126
</RiskyTruthyFalsyComparison>
11211127
</file>
11221128
<file src="src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php">
1129+
<ComplexMethod>
1130+
<code><![CDATA[analyze]]></code>
1131+
</ComplexMethod>
11231132
<ImplicitToStringCast>
11241133
<code><![CDATA[$method_id]]></code>
11251134
<code><![CDATA[$method_id]]></code>
@@ -1200,6 +1209,9 @@
12001209
</RiskyTruthyFalsyComparison>
12011210
</file>
12021211
<file src="src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php">
1212+
<ComplexMethod>
1213+
<code><![CDATA[handleArrayAccessOnKeyedArray]]></code>
1214+
</ComplexMethod>
12031215
<ImplicitToStringCast>
12041216
<code><![CDATA[$array_type]]></code>
12051217
<code><![CDATA[$array_type]]></code>
@@ -1305,6 +1317,9 @@
13051317
</RiskyTruthyFalsyComparison>
13061318
</file>
13071319
<file src="src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php">
1320+
<ComplexMethod>
1321+
<code><![CDATA[infer]]></code>
1322+
</ComplexMethod>
13081323
<RiskyTruthyFalsyComparison>
13091324
<code><![CDATA[$fq_classlike_name]]></code>
13101325
</RiskyTruthyFalsyComparison>
@@ -1489,6 +1504,11 @@
14891504
<code><![CDATA[empty($classlike_storage->overridden_method_ids[$method_name])]]></code>
14901505
</RiskyTruthyFalsyComparison>
14911506
</file>
1507+
<file src="src/Psalm/Internal/Codebase/CombinedFlowGraph.php">
1508+
<PossiblyUnusedMethod>
1509+
<code><![CDATA[addSink]]></code>
1510+
</PossiblyUnusedMethod>
1511+
</file>
14921512
<file src="src/Psalm/Internal/Codebase/ConstantTypeResolver.php">
14931513
<InvalidOperand>
14941514
<code><![CDATA[$left->value & $right->value]]></code>
@@ -2147,6 +2167,9 @@
21472167
</RiskyTruthyFalsyComparison>
21482168
</file>
21492169
<file src="src/Psalm/Internal/Type/Comparator/AtomicTypeComparator.php">
2170+
<ComplexMethod>
2171+
<code><![CDATA[isContainedBy]]></code>
2172+
</ComplexMethod>
21502173
<PossiblyUndefinedIntArrayOffset>
21512174
<code><![CDATA[$array->properties[0]]]></code>
21522175
<code><![CDATA[$array->properties[0]]]></code>
@@ -2271,6 +2294,9 @@
22712294
</RiskyTruthyFalsyComparison>
22722295
</file>
22732296
<file src="src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php">
2297+
<ComplexMethod>
2298+
<code><![CDATA[handleTemplateParamStandin]]></code>
2299+
</ComplexMethod>
22742300
<ImpureMethodCall>
22752301
<code><![CDATA[getClassTemplateTypes]]></code>
22762302
</ImpureMethodCall>
@@ -2318,6 +2344,9 @@
23182344
</RiskyTruthyFalsyComparison>
23192345
</file>
23202346
<file src="src/Psalm/Internal/Type/TypeExpander.php">
2347+
<ComplexMethod>
2348+
<code><![CDATA[expandConditional]]></code>
2349+
</ComplexMethod>
23212350
<InvalidArgument>
23222351
<code><![CDATA[$fallback_params]]></code>
23232352
</InvalidArgument>
@@ -2860,6 +2889,14 @@
28602889
<code><![CDATA[empty($normalizedEntry['type'])]]></code>
28612890
</RiskyTruthyFalsyComparison>
28622891
</file>
2892+
<file src="tests/ReportOutputTest.php">
2893+
<MixedAssignment>
2894+
<code><![CDATA[$issue_data]]></code>
2895+
</MixedAssignment>
2896+
<PossiblyFalseArgument>
2897+
<code><![CDATA[file_get_contents(__DIR__.'/sarif.json')]]></code>
2898+
</PossiblyFalseArgument>
2899+
</file>
28632900
<file src="tests/TestConfig.php">
28642901
<InvalidExtendClass>
28652902
<code><![CDATA[Config]]></code>

psalm.xml.dist

+2-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
name="Psalm for Psalm"
66
errorLevel="1"
77
noCache="true"
8-
forceJit="true"
8+
forceJit="false"
99
throwExceptionOnError="0"
1010
findUnusedCode="true"
1111
ensureArrayStringOffsetsExist="true"
@@ -16,6 +16,7 @@
1616
allFunctionsGlobal="true"
1717
ensureOverrideAttribute="true"
1818
allConstantsGlobal="true"
19+
runTaintAnalysis="true"
1920
findUnusedPsalmSuppress="true"
2021
findUnusedBaselineEntry="true"
2122
findUnusedIssueHandlerSuppression="true"

src/Psalm/Internal/Analyzer/ClosureAnalyzer.php

+4-5
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
use PhpParser;
99
use Psalm\CodeLocation;
1010
use Psalm\Context;
11-
use Psalm\Internal\Codebase\VariableUseGraph;
1211
use Psalm\Internal\DataFlow\DataFlowNode;
1312
use Psalm\Internal\PhpVisitor\ShortClosureVisitor;
1413
use Psalm\Issue\DuplicateParam;
@@ -138,13 +137,13 @@ public static function analyzeExpression(
138137

139138
$use_var_id = '$' . $use->var->name;
140139

141-
if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph
140+
if ($statements_analyzer->variable_use_graph
142141
&& $context->hasVariable($use_var_id)
143142
) {
144143
$parent_nodes = $context->vars_in_scope[$use_var_id]->parent_nodes;
145144

146145
foreach ($parent_nodes as $parent_node) {
147-
$statements_analyzer->data_flow_graph->addPath(
146+
$statements_analyzer->variable_use_graph->addPath(
148147
$parent_node,
149148
DataFlowNode::getForClosureUse(),
150149
'closure-use',
@@ -184,11 +183,11 @@ public static function analyzeExpression(
184183
if ($context->hasVariable($use_var_id)) {
185184
$use_context->vars_in_scope[$use_var_id] = $context->vars_in_scope[$use_var_id];
186185

187-
if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph) {
186+
if ($statements_analyzer->variable_use_graph) {
188187
$parent_nodes = $context->vars_in_scope[$use_var_id]->parent_nodes;
189188

190189
foreach ($parent_nodes as $parent_node) {
191-
$statements_analyzer->data_flow_graph->addPath(
190+
$statements_analyzer->variable_use_graph->addPath(
192191
$parent_node,
193192
DataFlowNode::getForClosureUse(),
194193
'closure-use',

src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php

+26-33
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
use Psalm\Internal\Analyzer\FunctionLike\ReturnTypeCollector;
2121
use Psalm\Internal\Analyzer\Statements\Expression\Call\FunctionCallReturnTypeFetcher;
2222
use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer;
23-
use Psalm\Internal\Codebase\TaintFlowGraph;
24-
use Psalm\Internal\Codebase\VariableUseGraph;
2523
use Psalm\Internal\DataFlow\DataFlowNode;
2624
use Psalm\Internal\FileManipulation\FileManipulationBuffer;
2725
use Psalm\Internal\FileManipulation\FunctionDocblockManipulator;
@@ -1070,39 +1068,36 @@ private function processParams(
10701068
if ($statements_analyzer->data_flow_graph
10711069
&& $function_param->location
10721070
) {
1073-
//don't add to taint flow graph if the type can't transmit taints
1074-
if (!$statements_analyzer->data_flow_graph instanceof TaintFlowGraph
1075-
|| $function_param->type === null
1076-
|| !$function_param->type->isSingle()
1077-
|| (!$function_param->type->isInt()
1078-
&& !$function_param->type->isFloat()
1079-
&& !$function_param->type->isBool())
1080-
) {
1081-
$param_assignment = DataFlowNode::getForAssignment(
1082-
$function_param_id,
1083-
$function_param->location,
1084-
);
1085-
1086-
$statements_analyzer->data_flow_graph->addNode($param_assignment);
1071+
$param_assignment = DataFlowNode::getForAssignment(
1072+
$function_param_id,
1073+
$function_param->location,
1074+
);
10871075

1088-
if ($cased_method_id !== null) {
1089-
$type_source = DataFlowNode::getForMethodArgument(
1090-
$cased_method_id,
1091-
$cased_method_id,
1092-
$offset,
1093-
$function_param->location,
1094-
null,
1095-
);
1076+
$statements_analyzer->data_flow_graph->addNode($param_assignment);
10961077

1097-
$statements_analyzer->data_flow_graph->addPath($type_source, $param_assignment, 'param');
1098-
}
1078+
if ($cased_method_id !== null) {
1079+
$type_source = DataFlowNode::getForMethodArgument(
1080+
$cased_method_id,
1081+
$cased_method_id,
1082+
$offset,
1083+
$function_param->location,
1084+
null,
1085+
);
10991086

1100-
if ($storage->variadic) {
1101-
$this->param_nodes += [$param_assignment->id => $param_assignment];
1102-
}
1087+
$statements_analyzer->data_flow_graph->addPath(
1088+
$type_source,
1089+
$param_assignment,
1090+
'param',
1091+
0,
1092+
$function_param->signature_type?->getTaintsToRemove() ?? 0,
1093+
);
1094+
}
11031095

1104-
$parent_nodes = [$param_assignment->id => $param_assignment];
1096+
if ($storage->variadic) {
1097+
$this->param_nodes += [$param_assignment->id => $param_assignment];
11051098
}
1099+
1100+
$parent_nodes = [$param_assignment->id => $param_assignment];
11061101
}
11071102

11081103
if ($function_param->type) {
@@ -2211,9 +2206,7 @@ private function detectUnusedParameters(
22112206

22122207
$assignment_node = DataFlowNode::getForAssignment($var_name, $original_location);
22132208

2214-
if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph
2215-
&& $statements_analyzer->data_flow_graph->isVariableUsed($assignment_node)
2216-
) {
2209+
if ($statements_analyzer->variable_use_graph?->isVariableUsed($assignment_node)) {
22172210
continue;
22182211
}
22192212

src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ public static function analyze(
428428
$codebase,
429429
);
430430

431-
if (!$combined_type->equals($context->vars_in_scope[$var_id])) {
431+
if (!$combined_type->equals($context->vars_in_scope[$var_id], true, false)) {
432432
$context->removeDescendents($var_id, $combined_type);
433433
}
434434

src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php

+1-4
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use Psalm\Internal\Analyzer\ClassLikeNameOptions;
1212
use Psalm\Internal\Analyzer\ScopeAnalyzer;
1313
use Psalm\Internal\Analyzer\StatementsAnalyzer;
14-
use Psalm\Internal\Codebase\VariableUseGraph;
1514
use Psalm\Internal\DataFlow\DataFlowNode;
1615
use Psalm\Internal\Scope\FinallyScope;
1716
use Psalm\Issue\InvalidCatch;
@@ -310,13 +309,11 @@ public static function analyze(
310309
])
311310
;
312311

313-
if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph) {
314-
$statements_analyzer->data_flow_graph->addPath(
312+
$statements_analyzer->variable_use_graph?->addPath(
315313
$catch_var_node,
316314
DataFlowNode::getForVariableUse(),
317315
'variable-use',
318316
);
319-
}
320317
}
321318
}
322319

src/Psalm/Internal/Analyzer/Statements/EchoAnalyzer.php

+2-3
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use Psalm\Internal\Analyzer\Statements\Expression\Call\ArgumentAnalyzer;
1212
use Psalm\Internal\Analyzer\Statements\Expression\CastAnalyzer;
1313
use Psalm\Internal\Analyzer\StatementsAnalyzer;
14-
use Psalm\Internal\Codebase\TaintFlowGraph;
1514
use Psalm\Internal\DataFlow\DataFlowNode;
1615
use Psalm\Issue\ForbiddenCode;
1716
use Psalm\Issue\ImpureFunctionCall;
@@ -44,7 +43,7 @@ public static function analyze(
4443

4544
$expr_type = $statements_analyzer->node_data->getType($expr);
4645

47-
if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
46+
if ($statements_analyzer->taint_flow_graph) {
4847
if ($expr_type && $expr_type->hasObjectType()) {
4948
$expr_type = CastAnalyzer::castStringAttempt(
5049
$statements_analyzer,
@@ -70,7 +69,7 @@ public static function analyze(
7069
);
7170

7271

73-
$statements_analyzer->data_flow_graph->addSink($echo_param_sink);
72+
$statements_analyzer->taint_flow_graph->addSink($echo_param_sink);
7473
}
7574

7675
if (ArgumentAnalyzer::verifyType(

0 commit comments

Comments
 (0)