search

xwikisas / application-antivirus

Keeps your XWiki instance safe by scanning file attachments for viruses or malware.
GNU Lesser General Public License v2.1
0 stars 1 forks source link

If the connection to the server times out, it uploads the file nevertheless and does not notify about the issue #18

Open iuliabalan opened 5 years ago

iuliabalan commented 5 years ago

We encountered this issue at a client's production (on which the timeout caused extra delay in uploading attachments). Thanks to @lucaa who noticed that the way the antivirus extension is working is that if the connection to the server times out, it uploads the file nevertheless and does not notify about the issue - this is not really OK, there should be a mention at least in the incidents log about the fact that the attachment could not be scanned.

Enygma2002 commented 5 years ago

No incident log is produced for this, as it's not a security incident, but an operations/configuration issue.

For the scheduled scan (i.e. the job), any scan errors, additionally to being logged, is also added to the list of scan errors that are included in the email report. During upload scan, any scan exception (scan error, server communication error, etc.) is logged, but the upload operation is not prevented.

Theoretically, if anything slips at upload time (either due to temporary AV server outage, not up to date virus definitions, etc.) should be caught by the scheduled scan. Canceling the document save (at attachment upload) because there was a scan error is too intrusive, IMO, and risks breaking too much functionality of the wiki.

Note that this case might be better covered by #7, allowing admins to check the configuration and status of the AV server, thus helping admins debug/solve the usecase of the current issue as well.

We could consider an improvement of notifying admins for each scan error or communication issue, but, IMO, that's also unnecessarily intrusive and annoying and will become and unused feature quickly.

lucaa commented 5 years ago

I agree that it can be a valid choice to let the attachments pass regardless of the possibility to check the attachments or not. It would be interesting, though, to have this as a configuration, I can totally see how this is something that an admin may want to configure (use a "strict" policy or a "confidence" policy with verification a-posteriori).

However, I would say that the fact that the check cannot be done for an attachment can be considered an security incident, not of the same type as the other incidents, but the fact that the "shield is down" is an incident per-se, regardless of whether an attack happened or not.