Skip to content

Fix heap over-read in cli_get_prompt() for empty cli.prompt - #23415

Closed
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/readline-cli-prompt-empty
Closed

iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/readline-cli-prompt-empty

Conversation

@iliaal

@iliaal iliaal commented Aug 22, 2026

Copy link
Copy Markdown
Member

cli_get_prompt() parsed cli.prompt with a do-while, so the empty string executed the body on the terminator and scanned heap past it. Use a while loop and return the interned empty string when nothing was appended.

@ndossche ndossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Almost right imo

Comment thread ext/readline/readline_cli.c Outdated
Comment on lines 203 to 207
smart_str_0(&retval);
if (!retval.s) {
return ZSTR_EMPTY_ALLOC();
}
return retval.s;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This entire block should be replaced with:

Suggested change
smart_str_0(&retval);
if (!retval.s) {
return ZSTR_EMPTY_ALLOC();
}
return retval.s;
return smart_str_extract(&retval);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

The prompt parser ran as a do-while, so an empty cli.prompt executed
the body on the terminator and scanned past it. Use a while loop and
smart_str_extract(), which returns the interned empty string when
nothing was appended. No regression test: extra unicode warnings from
the over-read depend on heap contents, so a .phpt cannot pin the bug
red-before.

Closes phpGH-23415
@iliaal
iliaal force-pushed the fix/readline-cli-prompt-empty branch from 78a6f6a to 0401773 Compare September 18, 2026 19:49
@iliaal iliaal closed this in c2c01ae Sep 19, 2026
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.

2 participants