-
Notifications
You must be signed in to change notification settings - Fork 712
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Allow clamd
to start normally when a whitelist is present.
#1309
fix: Allow clamd
to start normally when a whitelist is present.
#1309
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on a solution to this bug.
The same issue occurs when adding individual signature files outside of a CVD or CLD archive. E.g. if you drop in "clamav.hdb" file containing a hash-based signature, then clamd
loading will still fail with:
❯ ./install/sbin/clamd -F
LibClamAV Error: cli_cvdverify: Can't read CVD header
LibClamAV Error: cl_cvdgetage: cvdgetfileage() failed for /home/micah/workspace/clamav-micah-5/build/install/share/clamav/clamav.hdb
So rather than having cl_cvdgetage
ignore specific extensions like ".ign2
", it would be better to only verify for ".cvd
" and ".cld
".
Yeah, I'm not familiar with all of the extensions so the clarification is more than welcome. |
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? |
I think that you are correct. I noticed that In case you think it would be better to keep the behavior as it was, just say so and I will change it accordingly. |
I wasn't able to reproduce this, or else I don't fully understand what you mean. I believe it should fail if the 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. |
ca96f27
to
2c7860f
Compare
Clamscan and ClamD will throw an error if you use the '--fail-if-cvd-older-than=DAYS' / 'FailIfCvdOlderThan' option and try to load any plaintext signature files. That is, it throws an error when encountering plain signature files like `.ign2`, `.ldb`, `.hdb`, etc. This feature should only verify CVD / CLD files. The feature (and bug) was introduced in ClamAV 1.1.0, here: Cisco-Talos@e4fe665 With this change, the `cl_cvdgetage` checks will skip any file that is not a CVD or CLD. Fixes: Cisco-Talos#1174
2c7860f
to
9a7b186
Compare
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 ofclamd
.