jonof / moodle-antivirus_savdi Goto Github PK

@jonof are you aware of this?

https://community.sophos.com/free-antivirus-tools-for-desktops/f/discussions/121788/sav-for-linux-free-edition-is-discontinued

https://github.com/jonof/moodle-antivirus_savdi/blob/master/classes/scanner.php#L95

Here we decide on what the behaviour is when an error is found. This will incorrectly allow files onto the system, as Moodles antivirus manager only trips on SCAN_RESULT_FOUND here:

https://github.com/moodle/moodle/blob/master/lib/classes/antivirus/manager.php#L73

It should be OK -> SCAN_RESULT_ERROR, SCAN_RESULT_ERROR -> SCAN_RESULT_FOUND

Same for scandata

I really like the test client to check if everything is working. But I have antivirus_savdi | scannerisremote enabled and wondered what is wrong here:

At least until I noticed that for scannerisremote only the SCANDATA result is noteworthy.

Probably I should just disable the SCANFILE feature on SAVDID. But I didn't and I was very confused :-). So it might be helpful to indicate which of SCANDATA/SCANFILE is actually the interesting one (depending on scannerisremote).

As you are using stream_socket_client directly you should probably check Moodle's web proxy settings ($CFG->proxyhost, $CFG->proxyuser etc) and use them when the rquest is going out externally.

I don't think this is a blocker for approval in the plugins db at this stage because the plugin is usable on localhost without the proxy settings.

It would be ideal if the scanning notice was set in the case of an error or a virus being found, which can provide more information to the antivirus manager.

See antivirus_clamav:
https://github.com/moodle/moodle/blob/master/lib/antivirus/clamav/classes/scanner.php#L96
https://github.com/moodle/moodle/blob/master/lib/antivirus/clamav/classes/scanner.php#L205

In scanlocal(), client.php URL-encodes the file path.

Is this intended?

This plugin looks great, would be good to make it 'official'

We have been encountering a lot of scanner errors due to Data sent was shorter than expected. These seem to primarily occur with large video files from 100Mb to 1.4Gb. I haven't done a deep dive on it, but my best guess its just failing due to network flakiness occasionally. I propose we just have a configurable retry option similar to ClamAV, that will allow for these to be rerun before failing. While this might be a lot of extra time during the upload process for a user, it should generally be lower than the time taken to actually do the file upload in the first place.

I won't have capacity to fix this for the next few weeks, but I may be able to PR this in the next few months.

On [https://moodle.org/plugins/antivirus_savdi] the plugin is not yet listed as 3.9 compatible. I guess no serious changes (if any at all) have to be made for 3.9 compatibility, though I did not yet check if it works.

https://docs.moodle.org/dev/Check_API

ie do a raw-ish direct data only eicar submission, and make sure it connects, and detects the failure, and does so in some reasonable time limit (maybe < 1 sec for a pure data check)

This doesn't need to be run on every check, it could be cached for say 1 hour