- Notifications
You must be signed in to change notification settings - Fork8k
Fix GH-16322: imageaffine overflow on affine argument.#16334
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
bc91602 to2a070dcCompareext/gd/gd.c Outdated
| break; | ||
| caseIS_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, sinceZEND_LONG_EXCEEDS_INT() is a no-op there, but users can pass a matrix like[9223372036854775807., 1, 1, 1, 1, 1]. Same for theIS_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.
| caseIS_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 useZEND_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-- | ||
| <?phpif (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 ofPHP_INT_MIN andPHP_INT_MAX doubles would be used.
No description provided.