Skip to content

Truncate sscanf/fscanf format string at NUL byte before counting placeholders #5591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
phpstan-bot wants to merge 7 commits into from
Closed

Truncate sscanf/fscanf format string at NUL byte before counting placeholders #5591

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
25d50a1
Truncate `sscanf`/`fscanf` format string at NUL byte before counting …
phpstan-bot May 3, 2026
c4b7175
Add missing `%n`, `%i`, `%X` specifiers to sscanf return type inference
phpstan-bot May 3, 2026
16b970d
Add missing `%g` and `%D` sscanf specifiers to return type inference …
phpstan-bot May 3, 2026
24cc3b7
Fix `%u` sscanf specifier return type to `int|string`
phpstan-bot May 3, 2026
fea9027
Add comprehensive sscanf format validation test with C ValidateFormat…
phpstan-bot May 4, 2026
f7a725d
Handle `l`/`L`/`h` scanf size modifiers in format parsing
phpstan-bot May 4, 2026
ab39be5
Remove massive SscanfFormatValidationTest
phpstan-bot May 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/Rules/Functions/PrintfHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use function max;
use function sprintf;
use function strlen;
use function strstr;
use const PREG_SET_ORDER;

#[AutowiredService]
Expand Down Expand Up @@ -45,6 +46,13 @@ public function getScanfPlaceholdersCount(string $format): ?int
*/
private function parsePlaceholders(string $specifiersPattern, string $format, bool $isScanf): ?array
{
if ($isScanf) {
$beforeNul = strstr($format, "\0", true);
if ($beforeNul !== false) {
$format = $beforeNul;
}
}

$addSpecifier = '';
if ($this->phpVersion->supportsHhPrintfSpecifier()) {
$addSpecifier .= 'hH';
Expand Down
15 changes: 10 additions & 5 deletions src/Type/Php/SscanfFunctionDynamicReturnTypeExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use function count;
use function in_array;
use function preg_match_all;
use function strstr;

#[AutowiredService]
final class SscanfFunctionDynamicReturnTypeExtension implements DynamicFunctionReturnTypeExtension
Expand Down Expand Up @@ -48,9 +49,15 @@ public function getTypeFromFunctionCall(
return null;
}

if (preg_match_all('/%(\d*)(\[[^\]]+\]|[cdeEfosux]{1})/', $formatType->getValue(), $matches) > 0) {
$arrayBuilder = ConstantArrayTypeBuilder::createEmpty();
$formatValue = $formatType->getValue();
$beforeNul = strstr($formatValue, "\0", true);
if ($beforeNul !== false) {
$formatValue = $beforeNul;
}

$arrayBuilder = ConstantArrayTypeBuilder::createEmpty();

if (preg_match_all('/%(\d*)(\[[^\]]+\]|[cdeEfosux]{1})/', $formatValue, $matches) > 0) {
Copy link
Copy Markdown
Contributor

@hakre hakre May 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#hakre memo

Copy link
Copy Markdown
Collaborator Author

@phpstan-bot phpstan-bot May 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed. Here's a summary of what was done to address hakre's review:

Changes made:

  1. SscanfFunctionDynamicReturnTypeExtension.php — Added missing scanf specifiers %i (integer with base detection), %n (characters consumed count), and %X (uppercase hex) to both the regex pattern and the IntegerType mapping. These were already supported in PrintfHelper for parameter count validation but were missing from the return type inference.

  2. Test coverage for hakre's edge cases — Added tests verifying:

    • Empty format string → array{}|null
    • %n specifier → array{int|null}|null
    • %% literal percent → array{}|null
    • %i specifier → array{int|null}|null
    • %X specifier → array{int|null}|null
    • Mixed %d%n → array{int|null, int|null}|null
    • Rule tests for %n, %%, %i parameter count validation

Regarding hakre's suggestion to use sscanf('', $format) instead of NUL truncation: The NUL truncation approach is preferred because (a) the return type extension needs to know individual specifier types (%d → int, %s → string, etc.), which sscanf() doesn't provide, and (b) calling sscanf at analysis time would mix runtime execution with static analysis and require error suppression for malformed format strings. The truncation is a simple, correct pre-processing step that handles the NUL byte behavior without those downsides.

for ($i = 0; $i < count($matches[0]); $i++) {
$length = $matches[1][$i];
$specifier = $matches[2][$i];
Expand Down Expand Up @@ -81,11 +88,9 @@ public function getTypeFromFunctionCall(
$type = TypeCombinator::addNull($type);
$arrayBuilder->setOffsetValueType(new ConstantIntegerType($i), $type);
}

return TypeCombinator::addNull($arrayBuilder->getArray());
}

return null;
return TypeCombinator::addNull($arrayBuilder->getArray());
}

}
19 changes: 19 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-14567.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

namespace Bug14567;

use function PHPStan\Testing\assertType;

function sscanfNulTerminator(string $s) {
// NUL byte terminates sscanf format string - placeholders after \0 are ignored
assertType('array{int|null}|null', sscanf($s, "%d\0%d"));
assertType('array{int|null, string|null}|null', sscanf($s, "%d %s\0%d"));
assertType('array{}|null', sscanf($s, "\0%d%s"));
}

function fscanfNulTerminator($r) {
// Same for fscanf
assertType('array{int|null}|null', fscanf($r, "%d\0%d"));
assertType('array{int|null, string|null}|null', fscanf($r, "%d %s\0%d"));
assertType('array{}|null', fscanf($r, "\0%d%s"));
}
5 changes: 5 additions & 0 deletions tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,4 +147,9 @@ public function testBug10260(): void
$this->analyse([__DIR__ . '/data/bug-10260.php'], []);
}

public function testBug14567(): void
{
$this->analyse([__DIR__ . '/data/bug-14567.php'], []);
}

}
18 changes: 18 additions & 0 deletions tests/PHPStan/Rules/Functions/data/bug-14567.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

namespace Bug14567;

// NUL byte terminates sscanf/fscanf format string parsing
// Placeholders after \0 should not be counted

// Only 1 placeholder active before NUL
sscanf('123 456', "%d\0%d", $a);

// Only 1 placeholder active before NUL (fscanf)
fscanf(STDIN, "%d\0%d", $a2);

// No placeholders after NUL
sscanf('123', "\0%d");

// Multiple placeholders, NUL in middle
sscanf('123 456 789', "%d %d\0%d", $b, $c);
Loading