CLAM 1525: Fixed freshclam network path renaming bug

[kang-grace]

Fixed freshclam bug that, when using network paths, wouldn't allow temp files to be renamed. Cause: rename() doesn't work on Windows if the destination file already exists.
Redefined rename() to w32_rename() on Windows.

[micahsnyder]

Note: This fixes #197

[micahsnyder]

I've done some testing with this and it seems like a solid fix.

Before:

❯ .\freshclam.exe --datadir=\\localhost\c$\Users\micasnyd\workspace\test-tmp\
ClamAV update process started at Wed Aug  4 11:49:49 2021
daily database available for download (remote version: 26253)
Time:    3.5s, ETA:    0.0s [========================>]   55.05MiB/55.05MiB
Testing database: '\\localhost\c$\Users\micasnyd\workspace\test-tmp\\tmp.ef3083570e\clamav-4aa89903b0df487533f6e75f84d94ed1.tmp-daily.cvd' ...
Database test passed.
daily.cvd updated (version: 26252, sigs: 1966019, f-level: 90, builder: raynman)
Received an older daily CVD than was advertised. We'll retry so the incremental update will ensure we're up-to-date.
daily database available for download (remote version: 26253)
Time:    3.5s, ETA:    0.0s [========================>]   55.05MiB/55.05MiB
Testing database: '\\localhost\c$\Users\micasnyd\workspace\test-tmp\\tmp.ef3083570e\clamav-196bcfef82e1424cd8d36a717c991724.tmp-daily.cvd' ...
Database test passed.
ERROR: updatedb: Can't rename \\localhost\c$\Users\micasnyd\workspace\test-tmp\\tmp.ef3083570e\clamav-196bcfef82e1424cd8d36a717c991724.tmp-daily.cvd to daily.cvd: File exists
ERROR: Unexpected error when attempting to update daily: Failed to read/write file to database directory
ERROR: Database update process failed: Failed to read/write file to database directory
ERROR: Update failed.

After:

clamav-micah/build on  PR-226 via △ v3.21.1 took 22s
❯ .\install\freshclam.exe --datadir=\\localhost\c$\Users\micasnyd\workspace\test-tmp\
ClamAV update process started at Wed Aug  4 11:51:23 2021
daily database available for download (remote version: 26253)
Time:    3.6s, ETA:    0.0s [========================>]   55.05MiB/55.05MiB
Testing database: '\\localhost\c$\Users\micasnyd\workspace\test-tmp\\tmp.9d145d3059\clamav-12169a138ecf3d61693a281b17a22df8.tmp-daily.cvd' ...
Database test passed.
daily.cvd updated (version: 26252, sigs: 1966019, f-level: 90, builder: raynman)
Received an older daily CVD than was advertised. We'll retry so the incremental update will ensure we're up-to-date.
daily database available for download (remote version: 26253)
Time:    3.5s, ETA:    0.0s [========================>]   55.05MiB/55.05MiB
Testing database: '\\localhost\c$\Users\micasnyd\workspace\test-tmp\\tmp.9d145d3059\clamav-012ad366dde8971ee046e47b8d1dba2d.tmp-daily.cvd' ...
Database test passed.
daily.cvd updated (version: 26252, sigs: 1966019, f-level: 90, builder: raynman)
Received an older daily CVD than was advertised. We'll retry so the incremental update will ensure we're up-to-date.
daily database available for download (remote version: 26253)
Time:    3.5s, ETA:    0.0s [========================>]   55.05MiB/55.05MiB
Testing database: '\\localhost\c$\Users\micasnyd\workspace\test-tmp\\tmp.9d145d3059\clamav-6867e4b486d2f3fa859412934915b247.tmp-daily.cvd' ...
Database test passed.
daily.cvd updated (version: 26252, sigs: 1966019, f-level: 90, builder: raynman)
Received an older daily CVD than was advertised. We'll retry so the incremental update will ensure we're up-to-date.
main database available for download (remote version: 61)
Time:    9.7s, ETA:    0.0s [========================>]  160.41MiB/160.41MiB
Testing database: '\\localhost\c$\Users\micasnyd\workspace\test-tmp\\tmp.9d145d3059\clamav-326d15b54d5a4c946bc32dc0b0efe1a1.tmp-main.cvd' ...
Database test passed.
main.cvd updated (version: 61, sigs: 6607162, f-level: 90, builder: sigmgr)
bytecode database available for download (remote version: 333)
Time:    0.3s, ETA:    0.0s [========================>]  286.79KiB/286.79KiB
Testing database: '\\localhost\c$\Users\micasnyd\workspace\test-tmp\\tmp.9d145d3059\clamav-92da232dfb099b75347b6e18798d9325.tmp-bytecode.cvd' ...
Database test passed.
bytecode.cvd updated (version: 333, sigs: 92, f-level: 63, builder: awillia2)

I did notice some bad behavior from freshclam when using UNC paths and receiving an update that is 1 version behind though. See above "After". Instead of doing an incremental update by downloading and applying a patch file, it is downloading the entire database two more times. I'm not sure what's going wrong here. I will try to investigate a little further.

[micahsnyder]

As an example, this is what it should look like (but with UNC paths)

clamav-micah/build on  PR-226 via △ v3.21.1
❯ .\install\freshclam.exe --datadir=C:\Users\micasnyd\workspace\test-tmp-2\
ClamAV update process started at Wed Aug  4 12:00:05 2021
daily database available for download (remote version: 26253)
Time:    3.9s, ETA:    0.0s [========================>]   55.05MiB/55.05MiB
Testing database: 'C:\Users\micasnyd\workspace\test-tmp-2\\tmp.3e2f3a71e6\clamav-e79a8f759d4e15fe09af63382b1d883a.tmp-daily.cvd' ...
Database test passed.
daily.cvd updated (version: 26252, sigs: 1966019, f-level: 90, builder: raynman)
Received an older daily CVD than was advertised. We'll retry so the incremental update will ensure we're up-to-date.
daily database available for update (local version: 26252, remote version: 26253)
Current database is 1 version behind.
Downloading database patch # 26253...
Time:    0.1s, ETA:    0.0s [========================>]   12.83KiB/12.83KiB
Testing database: 'C:\Users\micasnyd\workspace\test-tmp-2\\tmp.3e2f3a71e6\clamav-86ef197fddbb590f9f905515a151043a.tmp-daily.cld' ...
Database test passed.

[micahsnyder]

It looks like access() doesn't support UNC paths.

I added a test under freshclam_test.py that does an incremental update but using UNC path names and found it is failing to check the current database, here

Here's a patch file with the new test, if you're interested in trying it: windows-freshclam-unc-path-test.txt
Note that the patch also adds a couple of print statements to show where the access check fails.

The access() check failure causes it to try to download the whole database instead, thinking that it doesn't have a database to update incrementally. The reason the test fails is because the test doesn't actually serve up the test.cvd file since it expects the cdiff patch files to work.

We use access() quite a bit in libfreshclam and elsewhere in ClamAV. It makes me wonder what else is broken with UNC paths. Do you have any idea if we can replace access() with something that works with UNC paths? Does ClamWin by chance have a fix for this already?

[micahsnyder]

This seems to be working great now. Nice work!

[micahsnyder]

I also cherry-picked this into the rel/0.104 branch so it gets into the release