Skip to content
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

Unexpected nan value in ext/gd/libgd/gd_filter.c:386 #16255

Closed
YuanchengJiang opened this issue Oct 6, 2024 · 3 comments
Closed

Unexpected nan value in ext/gd/libgd/gd_filter.c:386 #16255

YuanchengJiang opened this issue Oct 6, 2024 · 3 comments

Comments

@YuanchengJiang
Copy link

Description

The following code:

<?php
function makeFilter($resource, $matrix, $offset = 1.0)
{
return imageconvolution($resource, $matrix, $fusion, $offset);
}
$edgeMatrix = array(array(1, 0, 1), array(0, 5, 0), array(1, 0, 1));
$im = imagecreatetruecolor(40, 40);
makeFilter($im, $edgeMatrix);

Resulted in this output:

/php-src/ext/gd/libgd/gd_filter.c:386:45: runtime error: -nan is outside the range of representable values of type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /php-src/ext/gd/libgd/gd_filter.c:386

PHP Version

PHP 8.4.0-dev

Operating System

ubuntu 22.04

@devnexen
Copy link
Member

devnexen commented Oct 6, 2024

seems the issue had been fixed upstream, issue only on the bundled version.

@cmb69
Copy link
Member

cmb69 commented Oct 6, 2024

Simplified reproducer:

$matrix = array(array(1, 0, 1), array(0, 5, 0), array(1, 0, 1));
$im = imagecreatetruecolor(40, 40);
imageconvolution($im, $matrix, 0, 1.0);

The problem is that we're passing zero as filter_div argument of gdImageConvolution() which doesn't seem to be supported be either our bundled libgd nor upstream.

Anyhow, I think we should catch that early, i.e.

 ext/gd/gd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ext/gd/gd.c b/ext/gd/gd.c
index 899dc05b3e..68ede9d023 100644
--- a/ext/gd/gd.c
+++ b/ext/gd/gd.c
@@ -3759,6 +3759,10 @@ PHP_FUNCTION(imageconvolution)
 			}
 		}
 	}
+	if (div == 0.0) {
+		zend_argument_value_error(3, "must not be zero");
+		RETURN_THROWS();
+	}
 	res = gdImageConvolution(im_src, matrix, (float)div, (float)offset);
 
 	if (res) {

I'm not sure whether checking for 0.0 is sufficient; we might need to some epsilon comparison. Or gdImageConvolution() would need some is_nan() checks (slower, and would only work for new versions).

@nielsdos
Copy link
Member

That's not enough:

  • 2.225E-307 is the smallest double, but will be converted to 0 when the cast happens, which triggers the same problem again
  • div can be non-finite, which triggers the problem as well (e.g. imageconvolution($im, $matrix, NAN, 1.0);)
  • the same issues happen with offset

I'll prepare a PR.

nielsdos added a commit to nielsdos/php-src that referenced this issue Dec 15, 2024
nielsdos added a commit that referenced this issue Dec 16, 2024
* PHP-8.3:
  Fix GH-16255: Unexpected nan value in ext/gd/libgd/gd_filter.c
nielsdos added a commit that referenced this issue Dec 16, 2024
* PHP-8.4:
  Fix GH-17140 (Assertion failure in JIT trace exit with ZEND_FETCH_DIM_FUNC_ARG)
  Fix GH-16255: Unexpected nan value in ext/gd/libgd/gd_filter.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants