Skip to content

ext/standard: Reject null bytes in parse_str()#21942

Merged
TimWolla merged 1 commit intophp:masterfrom
LamentXU123:bug-fix-10
May 4, 2026
Merged

ext/standard: Reject null bytes in parse_str()#21942
TimWolla merged 1 commit intophp:masterfrom
LamentXU123:bug-fix-10

Conversation

@LamentXU123
Copy link
Copy Markdown
Contributor

<?php
parse_str("a=1\0&b=2", $result);
print_r($result);

resulted in

Array
(
    [a] => 1
)

reproduce: https://3v4l.org/UF6Pu#v
This might be the last important occurrence in the standard extension :)

@TimWolla
Copy link
Copy Markdown
Member

TimWolla commented May 3, 2026

@LamentXU123 Can you make sure to use the PR title as the commit title for single-commit PRs in the future? That makes merging much easier, because GitHub implicitly uses the commit data for single-commit PRs.

@LamentXU123
Copy link
Copy Markdown
Contributor Author

GitHub implicitly uses the commit data for single-commit PRs.

Weird behavior....

Can you make sure to use the PR title as the commit title for single-commit PRs in the future?

Uh got it :)

Comment thread ext/standard/string.c

ZEND_PARSE_PARAMETERS_START(2, 2)
Z_PARAM_STRING(arg, arglen)
Z_PARAM_PATH(arg, arglen)

This comment was marked as low quality.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the implementation of parse_str is based on C str, and therefore if the parsed string contains NUL bytes it will truncates the trailing part. Now, we have already done the same fix of getenv and putenv. From this fix we can throw a ValueError to users instead of silently truncate the input.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see, it would be great to add a comment here, that the limitation is due php_default_treat_data not supporting an explicit input string length.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Uh yes.That'd improve readability. Humm, we have lots of similar fix out there so adding comments to this case alone seems not to be effective... Could you please send a PR after this is merged to add this line of comment to every similar fix we have done before (i.e. reject NUL bytes because it silently truncates stuff)? Thank in advance!

@TimWolla TimWolla merged commit 2f4a214 into php:master May 4, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants