-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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.
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
Fix GH-16322: imageaffine overflow on affine argument. #16334
Conversation
bc91602
to
2a070dc
Compare
ext/gd/gd.c
Outdated
break; | ||
case IS_DOUBLE: | ||
affine[i] = Z_DVAL_P(zval_affine_elem); | ||
if (ZEND_LONG_EXCEEDS_INT(affine[i])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this will work on 32bit platforms, since ZEND_LONG_EXCEEDS_INT()
is a no-op there, but users can pass a matrix like [9223372036854775807., 1, 1, 1, 1, 1]
. Same for the IS_STRING
case below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you re right I always forget.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Looks generally good to me, and I don't see any particular issues with constraining the range of the matrix elements.
@@ -3687,13 +3687,25 @@ PHP_FUNCTION(imageaffine) | |||
if ((zval_affine_elem = zend_hash_index_find(Z_ARRVAL_P(z_affine), i)) != NULL) { | |||
switch (Z_TYPE_P(zval_affine_elem)) { | |||
case IS_LONG: | |||
affine[i] = Z_LVAL_P(zval_affine_elem); | |||
affine[i] = Z_LVAL_P(zval_affine_elem); | |||
if (affine[i] < INT_MIN || affine[i] > INT_MAX) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This guard could use ZEND_LONG_EXCEEDS_INT()
, but for consistency it maybe better this way (and the compiler will optimize away anyway).
ext/gd/tests/gh16322.phpt
Outdated
--INI-- | ||
memory_limit=-1 | ||
--SKIPIF-- | ||
<?php if (PHP_INT_SIZE != 8) die('skip this test is for 64bit platforms only'); ?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test could make sense on 32bit platforms, too, if instead of PHP_INT_MIN
and PHP_INT_MAX
doubles would be used.
No description provided.