Fix #74960: Integer overflow in zend_string_alloc() #7252
Conversation
We need to check for potential integer overflow when allocating `zend_string`s. To be able to bail out, we need to forward declare `zend_error_noreturn()`.
| zend_string *ret; | ||
|
|
||
| if (UNEXPECTED(size < len)) { | ||
| zend_error_noreturn(E_ERROR, "Integer overflow in string allocation"); |
There was a problem hiding this comment.
It would be better to define a separate function for this (in zend_string.c).
|
For the particular bug report, isn't this covered by Line 1921 in 26fc7c4 |
|
I believe @dstogov was of the opinion that we should use zend_string_alloc_safe in cases where this is an issue. Personally I think it makes sense to do this check (and remove various workarounds we have introduced in other places, e.g. the one linked above). The check should be cheap and can be optimized away for constant strings. |
|
|
|
@dstogov, any thoughts on this issue? |
I don't like to add check into Especially for this bug, I suppose, |
|
Okay, that makes sense. Closing in favor of PR #7294. |
|
Expecting all users to check the size prior to calling
|
We need to check for potential integer overflow when allocating
zend_strings. To be able to bail out, we need to forward declarezend_error_noreturn().Note that there are more occurrences which would need to be patched accordingly, but I'm not sure whether this way of doing is good, and I'm not even sure if this will be fixed at all, since in practice it only affects 32bit systems, and even there is relevant only for pathological cases. I assume this is a known issue for ZE developers.