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

Is this a general problem with any image styles?

Is it only daemon mode which is affected?

Seeing the same issue the following changes in DaemonUnixSocket.php seem to work. Can't really say why though.. (have only tried daemon mode over unix socket. The executable mode works without any changes.)

changing line 39
from
fwrite($scanner_handler, pack("N", $bytes));
to
fwrite($scanner_handler, pack("N", fstat($file_handler)['size']);

OR

changing line 39
from
fwrite($scanner_handler, pack("N", $bytes));
to
fwrite($scanner_handler, pack("J", $bytes));

and line 43
from
fwrite($scanner_handler, pack("N", 0));
to
fwrite($scanner_handler, pack("J", 0));

I reproduce the same bug (in version 2.0) on my site with uploading an image with a higher resolution than validator constraint.

I tested the 2 propositions of @Bandana, only the first one works.

The ticket was created 2 years ago and the module 1releases no longer exist in version 1, I am switching the ticket to version 2 and submitting a patch.
It would be interesting to apply the same patch in DaemonUnixSocket.php.

Misprint in my patch, sorry.

D9 with Version 2.0.0
J instead of N = unsigned long long (always 64 bit, big endian byte order).

Still has the same issue.
I have tried with the two above solutions..
It's throwing the same error.

Changing the status.. to Review.
In my case, it's working fine with Daemon mode (over Unix socket) Socket path: /var/run/clamav/clamd.ctl

But, ClamAV can't get to listen on TCP 3310 (Debian 11). I think there is some issues with my Docker host. Cannot bind to port 3310
Finding the solution with TCP/IP...

Also getting a similar error - may be related to file size. However, the file I'm uploading is an audio/mp3.

Notice: stream_copy_to_stream(): send of 8192 bytes failed with errno=32 Broken pipe in Drupal\clamav\Scanner\DaemonUnixSocket->scan() (line 40 of /var/www/app/modules/contrib/clamav/src/Scanner/DaemonUnixSocket.php)

Notice: fwrite(): send of 4 bytes failed with errno=32 Broken pipe in Drupal\clamav\Scanner\DaemonUnixSocket->scan() (line 43 of /var/www/app/modules/contrib/clamav/src/Scanner/DaemonUnixSocket.php)

Uploaded file /tmp/phpTC5t5h could not be checked, and was deleted.

I can confirm ClamAV is installed and running properly. It occasionally throws such errors with certain files.

Using Daemon mode (over Unix socket) on Ubuntu 20.x and PHP 7.4

Hi,
This MR works on my dev stack, thanks ! I use the official clamav/clamav docker image with Daemon mode (over TCP/IP) (port 3310).
I use the MR patch : https://git.drupalcode.org/project/clamav/-/merge_requests/4.patch

I don't get any error at the moment.

For the record, here's how I try to debug problem:

The scans seemed to hit a timeout each time I send a image file that was too big ( > 100kB).

I try to analyse the fgets call to clamd and get a mysterious COMMAND READ TIMED OUT hence leading me to thinks about a clamd incorrect setting...

I now understand it was not linked to the file size but to its pixel size and image style.

the $file passed into DaemonTCPIP::scan() is actually a resized thumbnail of the original image.

So the correct fix should be to send the original file to clamav, no ? It seems like a serious security issue

This patch is not working, all files seems to be clean.

To reproduce :

1. Apply patch
2. Try this fake virus signature : https://gist.githubusercontent.com/mikecastrodemaria/0843f8828fef7c60558...

After some debug, I can confirm that "max_resolution" resize the file before hook_file_validate, but update the correct filesize in the $file object after that.

So the quick fix I use is to update size before scanning the file in the clamav_file_validate

    # Update file size in case of "max_resolution" resizing.
    $file->setSize(@filesize($file->getFileUri()));

    $result = $scanner->scan($file);

I'll will do a bit more testing before submitting a MR

Please review the core issue https://www.drupal.org/project/drupal/issues/3292350, if it's merged this issue could be closed (works as designed).

Hi,
I have the same issue. I tried to use the patch about the core issue, but it's throwing the same error. I have a patch with a new proposal, it could be better if we have bytes information of the temp file that it will be the file to scan. I hope it works.

Regards.

@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 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 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 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 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 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.

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

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

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.

Pushed a new PR targetting 2.1.x, there are a bunch of CS stuff that still needs addressing, but that's better left to be handled in a separate issue.

Also introduced a new DaemonBase class since the IP and UNIX daemon scanners have duplicated code.

This should make it easier to apply bugfixes for the daemons going forward.