Skip to content

Fix GH-13142: Undefined variable name is shortened when contains \0 #13200

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
wants to merge 6 commits into from
Closed

Fix GH-13142: Undefined variable name is shortened when contains \0 #13200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c006610
Use %S formatter where possible for undefined variables
nielsdos Jan 19, 2024
b02076f
Make php_verror compatible with strings containing \0
nielsdos Jan 19, 2024
beb4670
Let compact() use %S
nielsdos Jan 19, 2024
fd98afa
Add test for GH-13142
nielsdos Jan 19, 2024
07ed57a
Extra test
nielsdos Jan 20, 2024
3163b43
Try to avoid duplicate function bodies
nielsdos Jan 20, 2024
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
29 changes: 29 additions & 0 deletions Zend/tests/gh13142.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
--TEST--
GH-13142 (Undefined variable name is shortened when contains \0)
--FILE--
<?php

$a = "test\0test";
Copy link
Member

Choose a reason for hiding this comment

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

It might be better if there were also tests in the case of \0test and test\0

$$a;
$a = "\0test";
$$a;
$a = "test\0";
$$a;

compact("a\0b");
compact("\0ab");
compact("ab\0");

?>
--EXPECTF--
Warning: Undefined variable $test%0test in %s on line %d

Warning: Undefined variable $%0test in %s on line %d

Warning: Undefined variable $test%0 in %s on line %d

Warning: compact(): Undefined variable $a%0b in %s on line %d

Warning: compact(): Undefined variable $%0ab in %s on line %d

Warning: compact(): Undefined variable $ab%0 in %s on line %d
2 changes: 1 addition & 1 deletion Zend/tests/warning_during_heredoc_scan_ahead.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ Warning: Octal escape sequence overflow \400 is greater than \377 in %s on line

Deprecated: Using ${expr} (variable variables) in strings is deprecated, use {${expr}} instead in %s on line %d

Warning: Undefined variable $ in %s on line %d
Warning: Undefined variable $%0 in %s on line %d
54 changes: 29 additions & 25 deletions Zend/zend.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1597,26 +1597,22 @@ ZEND_API ZEND_COLD void zend_error_at(
va_end(args);
}

ZEND_API ZEND_COLD void zend_error(int type, const char *format, ...) {
zend_string *filename;
uint32_t lineno;
va_list args;
#define zend_error_impl(type, format) do { \
zend_string *filename; \
uint32_t lineno; \
va_list args; \
get_filename_lineno(type, &filename, &lineno); \
va_start(args, format); \
zend_error_va_list(type, filename, lineno, format, args); \
va_end(args); \
} while (0)

get_filename_lineno(type, &filename, &lineno);
va_start(args, format);
zend_error_va_list(type, filename, lineno, format, args);
va_end(args);
ZEND_API ZEND_COLD void zend_error(int type, const char *format, ...) {
zend_error_impl(type, format);
}

ZEND_API ZEND_COLD void zend_error_unchecked(int type, const char *format, ...) {
zend_string *filename;
uint32_t lineno;
va_list args;

get_filename_lineno(type, &filename, &lineno);
va_start(args, format);
zend_error_va_list(type, filename, lineno, format, args);
va_end(args);
zend_error_impl(type, format);
}

ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_at_noreturn(
Expand All @@ -1636,18 +1632,26 @@ ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_at_noreturn(
abort();
}

#define zend_error_noreturn_impl(type, format) do { \
zend_string *filename; \
uint32_t lineno; \
va_list args; \
get_filename_lineno(type, &filename, &lineno); \
va_start(args, format); \
zend_error_va_list(type, filename, lineno, format, args); \
va_end(args); \
/* Should never reach this. */ \
abort(); \
} while (0)

ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_noreturn(int type, const char *format, ...)
{
zend_string *filename;
uint32_t lineno;
va_list args;
zend_error_noreturn_impl(type, format);
}

get_filename_lineno(type, &filename, &lineno);
va_start(args, format);
zend_error_va_list(type, filename, lineno, format, args);
va_end(args);
/* Should never reach this. */
abort();
ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_noreturn_unchecked(int type, const char *format, ...)
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the point of this function, it is 1:1 as zend_error_noreturn above

Copy link
Member Author

Choose a reason for hiding this comment

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

The format attribute

Copy link
Contributor

@mvorisek mvorisek Jan 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I see, ZEND_ATTRIBUTE_FORMAT in .h

{
zend_error_noreturn_impl(type, format);
}

ZEND_API ZEND_COLD ZEND_NORETURN void zend_strerror_noreturn(int type, int errn, const char *message)
Expand Down
1 change: 1 addition & 0 deletions Zend/zend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ extern ZEND_API void (*zend_post_shutdown_cb)(void);

ZEND_API ZEND_COLD void zend_error(int type, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);
ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_noreturn(int type, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);
ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_noreturn_unchecked(int type, const char *format, ...);
/* For custom format specifiers like H */
ZEND_API ZEND_COLD void zend_error_unchecked(int type, const char *format, ...);
/* If filename is NULL the default filename is used. */
Expand Down
10 changes: 5 additions & 5 deletions Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4970,7 +4970,7 @@ static void zend_compile_static_var(zend_ast *ast) /* {{{ */
}

if (zend_hash_exists(CG(active_op_array)->static_variables, var_name)) {
zend_error_noreturn(E_COMPILE_ERROR, "Duplicate declaration of static variable $%s", ZSTR_VAL(var_name));
zend_error_noreturn_unchecked(E_COMPILE_ERROR, "Duplicate declaration of static variable $%S", var_name);
}

zend_eval_const_expr(&ast->child[1]);
Expand Down Expand Up @@ -7245,8 +7245,8 @@ static void zend_compile_closure_binding(znode *closure, zend_op_array *op_array

value = zend_hash_add(op_array->static_variables, var_name, &EG(uninitialized_zval));
if (!value) {
zend_error_noreturn(E_COMPILE_ERROR,
"Cannot use variable $%s twice", ZSTR_VAL(var_name));
zend_error_noreturn_unchecked(E_COMPILE_ERROR,
"Cannot use variable $%S twice", var_name);
}

CG(zend_lineno) = zend_ast_get_lineno(var_name_ast);
Expand Down Expand Up @@ -7378,8 +7378,8 @@ static void zend_compile_closure_uses(zend_ast *ast) /* {{{ */
int i;
for (i = 0; i < op_array->last_var; i++) {
if (zend_string_equals(op_array->vars[i], var_name)) {
zend_error_noreturn(E_COMPILE_ERROR,
"Cannot use lexical variable $%s as a parameter name", ZSTR_VAL(var_name));
zend_error_noreturn_unchecked(E_COMPILE_ERROR,
"Cannot use lexical variable $%S as a parameter name", var_name);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions Zend/zend_execute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ static zend_never_inline ZEND_COLD zval* zval_undefined_cv(uint32_t var EXECUTE_
{
if (EXPECTED(EG(exception) == NULL)) {
zend_string *cv = CV_DEF_OF(EX_VAR_TO_NUM(var));
zend_error(E_WARNING, "Undefined variable $%s", ZSTR_VAL(cv));
zend_error_unchecked(E_WARNING, "Undefined variable $%S", cv);
}
return &EG(uninitialized_zval);
}
Expand Down Expand Up @@ -3788,7 +3788,7 @@ static zend_never_inline void zend_fetch_this_var(int type OPLINE_DC EXECUTE_DAT
Z_ADDREF_P(result);
} else {
ZVAL_NULL(result);
zend_error(E_WARNING, "Undefined variable $this");
zend_error_unchecked(E_WARNING, "Undefined variable $this");
}
break;
case BP_VAR_IS:
Expand Down
8 changes: 4 additions & 4 deletions Zend/zend_vm_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1755,8 +1755,8 @@ ZEND_VM_C_LABEL(fetch_this):
/* Keep name alive in case an error handler tries to free it. */
zend_string_addref(name);
}
zend_error(E_WARNING, "Undefined %svariable $%s",
(opline->extended_value & ZEND_FETCH_GLOBAL ? "global " : ""), ZSTR_VAL(name));
zend_error_unchecked(E_WARNING, "Undefined %svariable $%S",
(opline->extended_value & ZEND_FETCH_GLOBAL ? "global " : ""), name);
if (type == BP_VAR_RW && !EG(exception)) {
retval = zend_hash_update(target_symbol_table, name, &EG(uninitialized_zval));
} else {
Expand All @@ -1778,8 +1778,8 @@ ZEND_VM_C_LABEL(fetch_this):
} else if (type == BP_VAR_IS || type == BP_VAR_UNSET) {
retval = &EG(uninitialized_zval);
} else {
zend_error(E_WARNING, "Undefined %svariable $%s",
(opline->extended_value & ZEND_FETCH_GLOBAL ? "global " : ""), ZSTR_VAL(name));
zend_error_unchecked(E_WARNING, "Undefined %svariable $%S",
(opline->extended_value & ZEND_FETCH_GLOBAL ? "global " : ""), name);
if (type == BP_VAR_RW && !EG(exception)) {
ZVAL_NULL(retval);
} else {
Expand Down
24 changes: 12 additions & 12 deletions Zend/zend_vm_execute.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions ext/opcache/jit/zend_jit_helpers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ static ZEND_COLD void ZEND_FASTCALL zend_jit_invalid_method_call(zval *object)
if (Z_TYPE_P(object) == IS_UNDEF && opline->op1_type == IS_CV) {
zend_string *cv = EX(func)->op_array.vars[EX_VAR_TO_NUM(opline->op1.var)];

zend_error(E_WARNING, "Undefined variable $%s", ZSTR_VAL(cv));
zend_error_unchecked(E_WARNING, "Undefined variable $%S", cv);
if (UNEXPECTED(EG(exception) != NULL)) {
return;
}
Expand Down Expand Up @@ -340,7 +340,7 @@ static int ZEND_FASTCALL zend_jit_undefined_op_helper(uint32_t var)
const zend_execute_data *execute_data = EG(current_execute_data);
zend_string *cv = EX(func)->op_array.vars[EX_VAR_TO_NUM(var)];

zend_error(E_WARNING, "Undefined variable $%s", ZSTR_VAL(cv));
zend_error_unchecked(E_WARNING, "Undefined variable $%S", cv);
return EG(exception) == NULL;
}

Expand All @@ -354,7 +354,7 @@ static int ZEND_FASTCALL zend_jit_undefined_op_helper_write(HashTable *ht, uint3
if (!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE)) {
GC_ADDREF(ht);
}
zend_error(E_WARNING, "Undefined variable $%s", ZSTR_VAL(cv));
zend_error_unchecked(E_WARNING, "Undefined variable $%S", cv);
if (!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE) && GC_DELREF(ht) != 1) {
if (!GC_REFCOUNT(ht)) {
zend_array_destroy(ht);
Expand Down Expand Up @@ -2418,7 +2418,7 @@ static void ZEND_FASTCALL zend_jit_invalid_property_incdec(zval *container, cons
if (Z_TYPE_P(container) == IS_UNDEF && opline->op1_type == IS_CV) {
zend_string *cv = EX(func)->op_array.vars[EX_VAR_TO_NUM(opline->op1.var)];

zend_error(E_WARNING, "Undefined variable $%s", ZSTR_VAL(cv));
zend_error_unchecked(E_WARNING, "Undefined variable $%S", cv);
}
if (opline->result_type & (IS_VAR|IS_TMP_VAR)) {
ZVAL_UNDEF(EX_VAR(opline->result.var));
Expand Down
8 changes: 4 additions & 4 deletions ext/opcache/jit/zend_jit_ir.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4646,7 +4646,7 @@ static int zend_jit_inc_dec(zend_jit_ctx *jit, const zend_op *opline, uint32_t o
if_def = jit_if_not_Z_TYPE(jit, op1_addr, IS_UNDEF);
ir_IF_FALSE_cold(if_def);

// zend_error(E_WARNING, "Undefined variable $%s", ZSTR_VAL(CV_DEF_OF(EX_VAR_TO_NUM(opline->op1.var))));
// zend_error_unchecked(E_WARNING, "Undefined variable $%S", CV_DEF_OF(EX_VAR_TO_NUM(opline->op1.var)));
ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_undefined_op_helper), ir_CONST_U32(opline->op1.var));

jit_set_Z_TYPE_INFO(jit, op1_def_addr, IS_NULL);
Expand Down Expand Up @@ -5588,7 +5588,7 @@ static int zend_jit_long_math_helper(zend_jit_ctx *jit,
if_def = jit_if_not_Z_TYPE(jit, op1_addr, IS_UNDEF);
ir_IF_FALSE_cold(if_def);

// zend_error(E_WARNING, "Undefined variable $%s", ZSTR_VAL(CV_DEF_OF(EX_VAR_TO_NUM(opline->op1.var))));
// zend_error_unchecked(E_WARNING, "Undefined variable $%S", CV_DEF_OF(EX_VAR_TO_NUM(opline->op1.var)));
ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_undefined_op_helper), ir_CONST_U32(opline->op1.var));

ref2 = jit_EG(uninitialized_zval);
Expand All @@ -5604,7 +5604,7 @@ static int zend_jit_long_math_helper(zend_jit_ctx *jit,
if_def = jit_if_not_Z_TYPE(jit, op2_addr, IS_UNDEF);
ir_IF_FALSE_cold(if_def);

// zend_error(E_WARNING, "Undefined variable $%s", ZSTR_VAL(CV_DEF_OF(EX_VAR_TO_NUM(opline->op2.var))));
// zend_error_unchecked(E_WARNING, "Undefined variable $%S", CV_DEF_OF(EX_VAR_TO_NUM(opline->op2.var)));
ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_undefined_op_helper), ir_CONST_U32(opline->op2.var));

ref2 = jit_EG(uninitialized_zval);
Expand Down Expand Up @@ -5980,7 +5980,7 @@ static int zend_jit_simple_assign(zend_jit_ctx *jit,
jit_SET_EX_OPLINE(jit, opline);

ZEND_ASSERT(Z_MODE(val_addr) == IS_MEM_ZVAL);
// zend_error(E_WARNING, "Undefined variable $%s", ZSTR_VAL(CV_DEF_OF(EX_VAR_TO_NUM(opline->op1.var))));
// zend_error_unchecked(E_WARNING, "Undefined variable $%S", CV_DEF_OF(EX_VAR_TO_NUM(opline->op1.var)));
ret = ir_CALL_1(IR_I32, ir_CONST_FC_FUNC(zend_jit_undefined_op_helper), ir_CONST_U32(Z_OFFSET(val_addr)));

if (check_exception) {
Expand Down
2 changes: 1 addition & 1 deletion ext/standard/array.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2608,7 +2608,7 @@ static void php_compact_var(HashTable *eg_active_symbol_table, zval *return_valu
zend_hash_update(Z_ARRVAL_P(return_value), Z_STR_P(entry), &data);
}
} else {
php_error_docref(NULL, E_WARNING, "Undefined variable $%s", ZSTR_VAL(Z_STR_P(entry)));
php_error_docref_unchecked(NULL, E_WARNING, "Undefined variable $%S", Z_STR_P(entry));
}
} else if (Z_TYPE_P(entry) == IS_ARRAY) {
if (Z_REFCOUNTED_P(entry)) {
Expand Down
Loading