Skip to content

Fix #79979: passing value to by-ref param via CUF(A) crashes#6000

Closed
cmb69 wants to merge 4 commits intophp:PHP-7.4from
cmb69:cmb/79979
Closed

Fix #79979: passing value to by-ref param via CUF(A) crashes#6000
cmb69 wants to merge 4 commits intophp:PHP-7.4from
cmb69:cmb/79979

Conversation

@cmb69
Copy link
Member

@cmb69 cmb69 commented Aug 17, 2020

Passing values to by-reference parameters is an error, if the function
is called directly. If the function is called via call_user_func()
or call_user_func_array(), however, only a warning is triggered.
Therefore we have to make sure to not call ZEND_TRY_ASSIGN_REF_*()
for non-references.

This is just an examplary fix for exec; sever other functions would need to be fixed the same way.

An alternative fix would be to make the ZEND_TRY_ASSIGN_REF_*() macros more liberal, e.g.

 Zend/zend_API.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Zend/zend_API.h b/Zend/zend_API.h
index 2a3b582902..99994bcc69 100644
--- a/Zend/zend_API.h
+++ b/Zend/zend_API.h
@@ -799,8 +799,9 @@ ZEND_API int zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval *zv, ze
 	_ZEND_TRY_ASSIGN_LONG(zv, lval, 0)
 
 #define ZEND_TRY_ASSIGN_REF_LONG(zv, lval) do { \
-	ZEND_ASSERT(Z_ISREF_P(zv)); \
-	_ZEND_TRY_ASSIGN_LONG(zv, lval, 1); \
+	if (Z_ISREF_P(zv)) { \
+		_ZEND_TRY_ASSIGN_LONG(zv, lval, 1); \
+	} \
 } while (0)
 
 #define _ZEND_TRY_ASSIGN_DOUBLE(zv, dval, is_ref) do { \

@cmb69
Fix #79979: passing value to by-ref param via CUF(A) crashes
04978b2 
Passing values to by-reference parameters is an error, if the function
is called directly.  If the function is called via `call_user_func()`
or `call_user_func_array()`, however, only a warning is triggered.
Therefore we have to make sure to not call `ZEND_TRY_ASSIGN_REF_*()`
for non-references.
@cmb69
Copy link
Member Author

cmb69 commented Aug 17, 2020

@dstogov, what do you think about this?

@cmb69 cmb69 added the Bug label Aug 17, 2020
@nikic
Copy link
Member

nikic commented Aug 17, 2020

This needs to be addressed in the implementation of zend_call_function. It needs to create a dummy reference wrapper in this case, similarly to what we do in normal SEND_* opcodes, even if we throw "cannot be passed by reference" warnings.

@cmb69
Copy link
Member Author

cmb69 commented Aug 17, 2020

Thanks Nikita! I'll have a look at this as soon as possible.

@cmb69 cmb69 marked this pull request as draft August 17, 2020 21:27
@cmb69
Don't send by-val if it is not allowed
5367c2f 
If a by-val send is not allowed, we must not do so.  Instead we wrap
the value in a temporarly created reference.
nikic
nikic reviewed Aug 24, 2020
View reviewed changes
@php-pulls php-pulls closed this in 6b6c2c0 Aug 24, 2020
@cmb69 cmb69 deleted the cmb/79979 branch August 24, 2020 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nikic nikic nikic approved these changes

Assignees

No one assigned

Labels

Bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cmb69 @nikic