Use of Max Resolution on Image Field causes ClamAV Timeout in Deamon mode

@ifcarrionc_globant. Strange, my core patch works on my website for some months now...

Your patch seems correct (do not trust the file object and reloading the current file size), but could you please test again mine just to check that I didn't miss something.
Thanks

I can confirm applying patch from #3292350 fixes this issue too

https://www.drupal.org/project/drupal/issues/3292350#comment-15324953 🐛 file_validate_image_resolution does not update file size after resizing Needs work worked for me.

Updated the MR incorporating @ifcarrionc_globant's latest fix, adding support for unix sockets as well.

Reading the filesize from the resource seems like the most stable way to address the issue.

The switch between "N" (unsigned long (always 32 bit, big endian byte order) and "J" (unsigned long long (always 64 bit, big endian byte order) doesn't appear to fit with the ClamAV docs for INSTREAM.

https://linux.die.net/man/8/clamd

The format of the chunk is: '[length][data]' where [length] is the size of the following data in bytes expressed as a 4 byte unsigned integer

Could this be specific to a particular ClamAV version?

It looks like the main branch of ClamAV is still using an unsigned 32bit value to calculate the chunk size – https://github.com/Cisco-Talos/clamav/blob/main/clamd/server-th.c#L830

I'd be extremely reluctant to change the pack definition from 32bit to 64bit without a thorough understanding of the root cause.

The core patch https://www.drupal.org/project/drupal/issues/3292350 🐛 file_validate_image_resolution does not update file size after resizing Needs work will be included in the next drupal release, it should fix this issue. Can some one check is there is still work on this issue? If so, maybe update the issue name and description or open a new issue?

I think you're right @manarth, I just attempted to incorporate the original approach suggested in the original @ThirstySix MR since I thought it was solving a different variation of the issue.

Using the 64bit "J" version lead to broken pipe issues:

Notice: stream_copy_to_stream(): Send of 424 bytes failed with errno=32 Broken pipe in Drupal\clamav\Scanner\DaemonTCPIP->scan()

Notice: fwrite(): Send of 8 bytes failed with errno=32 Broken pipe in Drupal\clamav\Scanner\DaemonTCPIP->scan()

And I've swiftly updated the MR as appropriate.

https://www.drupal.org/project/drupal/issues/3292350 🐛 file_validate_image_resolution does not update file size after resizing Needs work should fix the problem, is there other use case that prevent to close this issue?

This issue is back on ClamAV 2.1.0 that uses the new event instead of validation hook.

It seems resizing happens before that.

With https://www.drupal.org/project/drupal/issues/3292350 🐛 file_validate_image_resolution does not update file size after resizing Needs work applied, uploading works if too big image is uploaded and resized, but in 2.1.0, this is broken again

Any updates on this? The module does not seem to be usable if images are resized starting with version 2.1.0

Add the relation to the new core issue https://www.drupal.org/project/drupal/issues/3522463 🐛 FileValidationEvent may not report the correct file size Active with the switch from hook_file_validate to FileValidationEvent .

I think that until this core issue is fixed, $file->getSize(); should not be trusted and replaced by filesize($file->getFileUri()) on https://git.drupalcode.org/project/clamav/-/blob/2.1.x/src/Scanner/Daemo...

I'll try to provide a patch

Thanks, I will try both this and the branch on the other issue and report back

IMHO ClamAv should assure its FileImageDimensionsConstraintValidator get the higher priority since it should check original file for virus before any other manipulations (check, resizes, ...).
This is specially true for virus that target image resizing lib such as gd or imagick

I agree it would make sense to run ClamAV first. During resizing, a carefully crafted payload could already execute malicious code technically.

Anyways, I can confirm this is fixed by changing how the file size is read or by patching core by https://www.drupal.org/project/drupal/issues/3522463 🐛 FileValidationEvent may not report the correct file size Active .

Attaching a patch for this module.

Could someone review the last patch ?

I open a feature request for the priority point: https://www.drupal.org/project/clamav/issues/3540722 ✨ ClamAv must have the highest priority on FileValidation Event Active

I am not sure if it makes to apply this patch to the module, it is more of a workaround. I posted the patch for anyone needed a quick solution rather than as a suggestion to be merged to the module.

The right solution is https://www.drupal.org/project/drupal/issues/3522463 🐛 FileValidationEvent may not report the correct file size Active

I also agree with #39 that the ClamAV event subscriber should run before the resizing happens. I suggest going back to needs work and changing the order of subscribers, but this is not something I know how to implement.