phpdbg: possible NULL dereference of `param` in `PHPDBG_LIST(lines)` when called from `PHPDBG_COMMAND(list)`

[ushevchenko]

Description

PHPDBG_COMMAND(list) in sapi/phpdbg/phpdbg_prompt.c explicitly handles param == NULL and passes it to the lines handler:

PHPDBG_COMMAND(list) /* {{{ */
{
	if (!param) {
		return PHPDBG_LIST_HANDLER(lines)(PHPDBG_COMMAND_ARGS);
	} else switch (param->type) {
		...

PHPDBG_LIST(lines) in sapi/phpdbg/phpdbg_list.c then dereferences param without checking it:

PHPDBG_LIST(lines) /* {{{ */
{
	if (!PHPDBG_G(exec) && !zend_is_executing()) {
		phpdbg_error("Not executing, and execution context not set");
		return SUCCESS;
	}

	switch (param->type) {

So the code contains a path where param is NULL and is dereferenced. The !param check in the caller shows that its author considered this state possible, while the callee does not handle it.

Reachability. I did not reproduce a crash; this report is based on reading the code. The list command is registered with the argument specification "*", and phpdbg_internal_stack_execute() in phpdbg_cmd.c calls handler->handler(top) only if phpdbg_stack_verify() returned SUCCESS. For "*" the computed least is 1, so a call with no arguments fails with "expected at least ... arguments" before the handler runs. A search of sapi/phpdbg in master finds no direct call of PHPDBG_COMMAND_HANDLER(list), so PHPDBG_COMMAND(list) is reached only through this dispatcher. The !param branch therefore looks unreachable today.

Other command handlers are invoked directly with a NULL parameter (for example, PHPDBG_COMMAND_HANDLER(run)(NULL) in phpdbg.c), so a NULL param is an expected convention in phpdbg, and PHPDBG_LIST(lines) is the one that does not handle it. If the dead branch is intentional, removing it (or an assertion documenting the contract) would also resolve this report.

Suggested fix

 PHPDBG_LIST(lines) /* {{{ */
 {
+	if (!param) {
+		phpdbg_error("A line number or file is required");
+		return SUCCESS;
+	}
+
 	if (!PHPDBG_G(exec) && !zend_is_executing()) {

Found by Linux Verification Center (https://portal.linuxtesting.ru) using SVACE.
Author U. Shevchenko.

PHP Version

8.3.31 (static analysis of the source, not run); the same code is present in master

Operating System

N/A (static analysis)

[ndossche]

This is a false positive.
Your analysis is right that "*" yields least==1 so there's no issue.
The if (!param) branch in PHPDBG_COMMAND(list) is dead code. We can remove it.

[ushevchenko]

This is a false positive. Your analysis is right that "*" yields least==1 so there's no issue. The if (!param) branch in PHPDBG_COMMAND(list) is dead code. We can remove it.

Thanks for confirming. Removing the dead if (!param) branch in PHPDBG_COMMAND(list) sounds good to me.