nextcloud / files_antivirus

👾 Antivirus app for Nextcloud Files
https://apps.nextcloud.com/apps/files_antivirus
GNU Affero General Public License v3.0
84 stars 35 forks source link

ScannerFactory.getScanner should always return a fresh scanner instance. #204

Closed kesselb closed 3 years ago

kesselb commented 3 years ago

Fix #202

Summary: A file scanned after an infected files is also infected.

How to reproduce:

https://github.com/nextcloud/files_antivirus/blob/f3a55a98772e24e11384f52a1bd7f11e0bafe72a/lib/Scanner/ScannerBase.php#L150-L156

IScanner.initScanner is called by ScannerBase.scan, ScannerBase.scanString, ScannerBase.retry. If files_antivirus is connected to a scanner via socket it may happen that writing to the socket is not possible anymore. We have to reopen the socket to write more data. That seems the reason for the above copy the status when infected logic in ScannerBase.initScanner: Keep the current status when reopening the socket.

This pull request change ScannerFactory.getScanner to always return a fresh instance of a scanner object. An alternative approach might be to introduce a resetScanner method (to be called by ScannerBase.scan and Scanner.scanString) and reset $status, $infectedStatus, $writeHandle, $lastChunk, $isLogUsed, $isAborted.

I guess both ways should work fine. Yet with the current approach (fresh instance for each file) we have more instances of the scanner around when running the background scan.

kesselb commented 3 years ago

Thanks @bpatath for your analysis in https://github.com/nextcloud/files_antivirus/issues/167.