search

Cisco-Talos / clamav

ClamAV - Documentation is here: https://docs.clamav.net
https://www.clamav.net/
GNU General Public License v2.0
4.27k stars 692 forks source link

fix: Allow `clamd` to start normally when a whitelist is present. #1309

Closed userwiths closed 2 months ago

userwiths commented 2 months ago

This PR is an attempt to fix issue #1174

The given functionality was introduced in the following commit and after taking a look at the cli_cvdverify function I believe it is meant to verify only CVDs.

That is, it throws an error when encountering a clear text (whitelist, .ign,.ign2) file. I mimicked the way the allowed extensions in the Database folder were handled, and excluded the 2 clear text extensions mentioned above. This has locally allowed for the normal execution of clamd.

userwiths commented 2 months ago

Yeah, I'm not familiar with all of the extensions so the clarification is more than welcome. As discussed now the cl_cvdgetage checks only CVD and CLD files.

micahsnyder commented 2 months ago

Hi @userwiths I thought of one other issue. If anyone tries to run clamscan where they pass in specific databases rather than using a database directory, it will still fail.

E.g.:

❯ clamscan -d ~/clamav.hdb -d ~/.cvdupdate/database/bytecode.cvd --fail-if-cvd-older-than=5 /path/to/file
LibClamAV Error: cli_cvdverify: Can't read CVD header
ERROR: Broken or not a CVD file

I think to fix this, we'll need to check if the database path ends with ".cvd" or ".cld" in this location: https://github.com/userwiths/clamav/blob/8474e6a86d45ba2b084bd7d3719947db90aa5543/clamscan/manager.c#L1253-L1254

What do you think?

userwiths commented 2 months ago

I think that you are correct. Despite that, I saw a specific scenario for which I took a decision that might be worth a notice.

I noticed that clamscan proceeds without a notice even if a file/directory passed with -d does not exist. Don't know if that is intentional or not, my current changes exit with an error and log on the screen a message with the path to the missing file.

In case you think it would be better to keep the behavior as it was, just say so and I will change it accordingly.

micahsnyder commented 2 months ago

I noticed that clamscan proceeds without a notice even if a file/directory passed with -d does not exist.

I wasn't able to reproduce this, or else I don't fully understand what you mean. I believe it should fail if the -d file or directory does not exist. So I'm good with your change either way.

Your PR seems good to me. I was doing some local testing after a rebase. I'm going to squash it down to one commit as well and then force-push that.