ExifMetadataReader class fails due to unnecessary check for 'allow_url_fopen()'

[jcogs-design]

Issue description

A new Imagine instance can be generated by loading the content of the image file directly.
If you try and add ExifMetadataReader to an instance created this way, and the server environment has allow_url_fopen set to 0, the attempt fails: even though in this instance there is no need for remote access to anything to generate the image or read the metadata.

The issue is being felt on a server that has a 'locked down' security status (i.e. allow_url_fopen is set to 0, but curl is available) - even though in the specific use case no remote file access is required, if it was Curl is available and could be used. But the check carried out by ExifMetadataReader in getUnsupportedReason() doesn't consider either of these contexts, and throws the exception simply because of the value of allow_url_fopen which appears to be inappropriate.
...

What version of Imagine are you using?

1.3.5

What's the PHP version you are using?

8.3

What's the imaging library you are using [gd/imagick/gmagick/any]?

gd2

Minimal PHP code to reproduce the error:

Where $source_image_raw is the content of an image file (obtained from reading a local disk file, or from remote location).

$source_image = (new Imagine())->setMetadataReader(new \Imagine\Image\Metadata\ExifMetadataReader())->load($source_image_raw);

[ausi]

The ExifMetadataReader uses data:// URLs to read the data.
This only works if allow_url_fopen is enabled.
You can find more details about that here: #728

[jcogs-design]

Well thanks for the additional information.

Looks like this was causing problems in 2019, and is still causing problems now.

This is a pity - as this choice (to use data:// - and this as far as I can tell is the only use of data:// in the entire Imagine library) prevents code that uses Imagine to read metadata from running on any secured php server (I don't have stats, but it appears that disabling allow_url_fopen is a standard security change for production server and has been for a long time).

In my actual use case, suggesting to a sysadmin that they make their server less secure so that an image utility that has no actual need to contact a remote server can correct the rotation of an iPhone image (that was itself read from a local disk) seems like a request that is unlikely to be satisfied.

The use of the data:// wrapper is solely to accommodate exif_read_data()'s requirement that its source be a "file". Where allow_url_fopen is available, using data:// is the solution for reading image data that exists as a string value. However where allow_url_fopen is not available it is a poor choice, as it doesn't work - as the two issues relating to this on this library indicate.

I think the solution is, where enable_url_fopen is not available, to instead write the image data to a temporary file, use that to get the exif data and then delete the temporary file. I have written a demonstration of this principle working in a PR to the file ExifMetadataReader.php - but since I don't have great familiarity with how Imagine handles file I/O, and since the code's primary purpose is to demonstrate that alternatives that fix this issue can exist, I have written it using GD2 functions (which I can test to ensure the idea works) rather than provide a library independent version.

I hope the PR might encourage a more tolerant view of this issue: moving away from the view that it is some kind of "absolute" issue that cannot be addressed. Maybe also the PR might inspire someone who can write better code to fix this issue more generically.

[ausi]

I don't have stats, but it appears that disabling allow_url_fopen is a standard security change for production server

I don’t think this is true. If a production server should not be able to make network connections, this should be prevented from the operating system side as allow_url_fopen is not sufficient to do that.

But I agree that if possible, this library should not require allow_url_fopen to be enabled.

moving away from the view that it is some kind of "absolute" issue that cannot be addressed

I don’t see this as “absolute” in any way, this can totally be addressed I think. But I also think it would be great if this gets fixed in PHP itself as well, so maybe you could add a comment with your use case there: https://bugs.php.net/bug.php?id=47336

I think the solution is, where enable_url_fopen is not available, to instead write the image data to a temporary file

There might be an even better solution using streams instead of data:-URIs. This probably also leads to a better performance. I’ll try to work on a pull request for that.

[jcogs-design]

If a production server should not be able to make network connections, this should be prevented from the operating system side as allow_url_fopen is not sufficient to do that.

I completely agree it is not the only thing that is, could, or should be done. Nonetheless it does appear to be widely used as part of steps to secure php servers.

it would be great if this gets fixed in PHP itself as well, so maybe you could add a comment with your use case

Of course - but the issue has been open for 15 years and is cast as a documentation issue rather than a code issue, so I'm not expecting that it would lead to a useful code change any time soon.

There might be an even better solution using streams instead of data:-URIs. This probably also leads to a better performance. I’ll try to work on a pull request for that.

That would be a very good outcome - let me know if there is anything I can do to help.

[jcogs-design]

Comment added ... https://bugs.php.net/bug.php?id=47336

[ausi]

I’ll try to work on a pull request for that.

See #861