Skip to content
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

Rework clamd command protocol (insecure by design) #1169

Open
defnull opened this issue Feb 8, 2024 · 3 comments
Open

Rework clamd command protocol (insecure by design) #1169

defnull opened this issue Feb 8, 2024 · 3 comments

Comments

@defnull
Copy link

defnull commented Feb 8, 2024

Disclaimer: The described issues are public knowledge since at least 2008 and the Cisco Product Security Incident Response Team (PSIRT) suggested to discuss this openly when I reported the findings as a security issue again in late 2023. So I'll do that now. Yes, this is a duplicate of #922 and #347 but with a wider scope and more details.

Describe the bug

It's not a bug in clamd, but a design flaw in the command protocol implemented by clamd, more specifically a lack of authorization or separation of concerns.

Background: clamd listens on a local unix socket by default, which is usually world-writeable to allow local users to actually use the daemon and send commands e.g. to scan files. clamd can also be configured to listen to a TCP socket and provide functionality to network clients (e.g. Mailservers). The protocol is documented here.

The problem: Administrative commands (e.g. SHUTDOWN, RELOAD) are not separated from normal user commands (e.g. SCAN) and clamd offers no way to restrict administrative commands to authorized users (e.g. just root). Every local user (or network client) can shutdown clamd, which is not restarted automatically on most Linux distributions, and even if it is, takes a significant amount of time to load signatures and be operable again.

Why this is bad: This can obviously be abused by bad actors or malware to disable clamd (and on-access scans) before downloading or unpacking malicious payload and avoid detection. To me, this looks like a textbook example of a unprivileged local (or remote) denial-of-service or circumvention-of-security-measures vulnerability with a very low complexity -> CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:C/C:N/I:N/A:H (8.6 high). Cisco PSIRT disagreed and closed the report as "works as intended". Twice.

More issues:

  • The VERSION command reveals the exact version string to local users (or network clients) and tells an attacker if known vulnerabilities in the scanning engine could be abused. The STATS command helps an attacker to see if the attack is successful. This information should not be available to unauthorized remote (non-localhost) clients.
  • The RELOAD command causes significant load and also pauses on-demand scanning (not confirmed) for a significant amount of time.
  • When clamd is configured to listen to TCP, a remote attacker could use the SCAN, CONTSCAN, MULTISCAN or ALLMATCHSCAN commands to probe the local file system for existing files or cause high IO and CPU load. There is no reason why a remote (non-localhost) network client should be allowed to issue scans for local files.
  • The SHUTDOWN and RELOAD commands should not be required at all because both can also be triggered (in a secure way) via signals.

How to reproduce

$ sudo install -y clamav-daemon && freshclam
$ sudo /etc/init.d/clamav-daemon start
$ sudo /etc/init.d/clamav-daemon status
clamd is running.
$ echo "SHUTDOWN" | nc -U /var/run/clamav/clamd.ctl
$ sudo /etc/init.d/clamav-daemon status
clamd is not running ... failed!
$ clamdscan ~
ERROR: Could not connect to clamd on LocalSocket
/var/run/clamav/clamd.ctl: No such file or directory

Proposed solution

Short term:

  • Administrative commands such as SHUTDOWN and RELOAD should be restricted to the local unix socket and root only (or root and the current user of the clamd process) as already suggested in Check for root user when processing SHUTDOWN command. #347. There is no reason an unprivileged user or even a remote client should be able to issue those commands.
    • Platforms that do not support SO_PEERCRED or a similar mechanism to determine the client user context for a local connection (Windows?) should be ignored for now. Implement it for platforms that do support it. Move to a more secure general solution later.
  • Remote (non-localhost) TCP clients should not be allowed to scan local files (SCAN, CONTSCAN, MULTISCAN or ALLMATCHSCAN). I cannot think of any valid use case for this functionality. The VERSION command is also risky, but removing that may actually break existing software. Not sure.
  • Phase out the old protocol behavior via a config option. It would be totally fine to be backwards compatible by default for a while, but administrators and maintainers need a way to move to a more secure behavior as soon as it is implemented.

Long term:

  • Either split control sockets from normal command sockets, or implement TLS encryption and an AUTH command that unlocks administrative commands for the currently connected client (e.g. based on a configurable shared secret and HMAC challenge).
  • We have to break backwards compatibility anyway to solve this, so this would be a perfect opportunity to get rid of some technical dept and remove deprecated commands along the way.
@stephane-chazelas
Copy link

Thanks for that.

For the record, some corresponding/related downstreams issues:

@micahsnyder
Copy link
Contributor

Your proposed solutions (short and long) both seem like valid ideas with one exception:

TCP clients should not be allowed to scan local files (SCAN, CONTSCAN, MULTISCAN or ALLMATCHSCAN). I cannot think of any valid use case for this functionality. The VERSION command is also risky, but removing that may actually break existing software. Not sure.

I agree that remote clients should not be able to scan local files. Unfortunately, Windows lacks the unix/local socket option. Something similar could probably be done with named pipes. That would require some dev work.
Windows also lacks the --fdpass option. Something similar may be possible to transfer file handles -- but I am not 100% certain. The scanning clients may not have the required privileges to transfer handles to the clamd server.

I also think fd-passing should become the preferred / default way to request file scans when clamd is on the same machine. The --stream/INSTREAM option only really makes sense for remote connections, and since the TCP interface does not support TLS at this time, doing so is very risky. If breaking the interface, I think we should add an option to disable support for INSTREAM, and disable INSTREAM by default - or else focus on adding both encryption and authentication.

Finally, I am not a fan of the current protocol. I recently proposed an extension to allow passing scan options with scan commands in #1165, but it is still limited and difficult to extend. And the scan response lacks the ability to send additional metadata/context about the scan results. I would really overhaul / replace it entirely with something extensible both in the request and the response, while maintaining both the fdpass and streaming capabilities. I vaguely recall some issue with fd-passing on some file systems, however. So if we disabled local-file-scans entirely, that may be problematic.

@defnull
Copy link
Author

defnull commented Feb 8, 2024

I agree that remote clients should not be able to scan local files. Unfortunately, Windows ...

Windows does have unix-style sockets for a while now, but TCP clients could also be considered "local" if they connect via localhost, which would allow network clients to connect to the local daemon and still issue commands reserved for local clients. Not sure if Windows supports SO_PEERCRED (to authorize administrative commands) yet, but all other major platforms support it, so we could just ignore windows for now and improve the situation for platforms that allow it. I'll edit my proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants