Skip to content

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

Merged
val-ms merged 1 commit intoCisco-Talos:mainfrom
userwiths:issue-1174-FailIfCvdOlderThan-error-on-whitelist
Jul 23, 2024
Merged

fix: Allow clamd to start normally when a whitelist is present.#1309
val-ms merged 1 commit intoCisco-Talos:mainfrom
userwiths:issue-1174-FailIfCvdOlderThan-error-on-whitelist

Conversation

@userwiths
Copy link
Contributor

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.

Copy link
Contributor

@val-ms val-ms left a 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".

@userwiths
Copy link
Contributor Author

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.

@userwiths userwiths requested a review from val-ms July 16, 2024 15:20
@val-ms
Copy link
Contributor

val-ms commented Jul 17, 2024

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
Copy link
Contributor Author

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.

@val-ms
Copy link
Contributor

val-ms commented Jul 23, 2024

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.

@val-ms val-ms force-pushed the issue-1174-FailIfCvdOlderThan-error-on-whitelist branch from ca96f27 to 2c7860f Compare July 23, 2024 17:46
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
@val-ms val-ms force-pushed the issue-1174-FailIfCvdOlderThan-error-on-whitelist branch from 2c7860f to 9a7b186 Compare July 23, 2024 20:01
@val-ms val-ms merged commit 6c0d644 into Cisco-Talos:main Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants