Fix GH-16189: underflow on preg_match/preg_match_all start_offset. #16191
Conversation
ext/pcre/php_pcre.c
Outdated
| if (start_offset == LONG_MIN) { | ||
| zend_argument_value_error(5, "must be greater than " ZEND_LONG_FMT, LONG_MIN); | ||
| RETURN_THROWS(); | ||
| } |
There was a problem hiding this comment.
| if (start_offset == LONG_MIN) { | |
| zend_argument_value_error(5, "must be greater than " ZEND_LONG_FMT, LONG_MIN); | |
| RETURN_THROWS(); | |
| } | |
| if (start_offset == LONG_MIN) { | |
| start_offset == LONG_MIN + 1; | |
| } |
There was a problem hiding this comment.
not sure we should hide the issue
There was a problem hiding this comment.
I belive we should, user should be able to pass ZEND_LONG_MIN.
ext/pcre/php_pcre.c
Outdated
| @@ -1135,6 +1135,11 @@ static void php_do_pcre_match(INTERNAL_FUNCTION_PARAMETERS, int global) /* {{{ * | |||
| RETURN_FALSE; | |||
| } | |||
|
|
|||
| if (start_offset == LONG_MIN) { | |||
There was a problem hiding this comment.
Is it possible that LONG_MIN is smaller than the minimum value of zend_off_t on some platforms?
For 8.4 and master we could update the start_offset parameter of php_pcre_match_impl() to ssize_t since it's casted to size_t after negation, and use SSIZE_MIN here.
For stable branches we could use ZEND_LONG_MIN as zend_long is always the same as zend_off_t.
| if (start_offset == ZEND_LONG_MIN) { | ||
| zend_argument_value_error(5, "must be greater than " ZEND_LONG_FMT, ZEND_LONG_MIN); | ||
| RETURN_THROWS(); | ||
| } |
There was a problem hiding this comment.
Surely it should also be checked that it is not larger than ZEND_LONG_MAX ?
There was a problem hiding this comment.
How long can be larger than ZEND_LONG_MAX?
There was a problem hiding this comment.
@mvorisek is right on this, start_offset is a zend_long.
No description provided.