Skip to content

Commit 412fd53

Browse files
authored
ClamD: harden VirusEvent virus name expansion (#1765)
VirusEvent executes the configured command through /bin/sh -c. It previously expanded %v by appending the detection name directly into that command string, matching the older behavior for virus-name substitution. Loaded signature names are treated as trusted database content rather than attacker-controlled scanned-file input, so this change is intended as hardening. Still, replacing the shell interpolation avoids a footgun for deployments that load custom signatures and use VirusEvent. Match the existing %f behavior by replacing %v with a disabled-feature message instead of the virus name. The virus name remains available to VirusEvent scripts through CLAM_VIRUSEVENT_VIRUSNAME. Update the clamd configuration help, sample config, manpage source, and VirusEvent unit test to cover the environment-variable path. Credit: Yazdan Soltani Credit: Nir Yehoshua CLAM-2992
1 parent a937323 commit 412fd53

6 files changed

Lines changed: 26 additions & 21 deletions

File tree

clamd/clamd_others.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ void virusaction(const char *filename, const char *virname,
102102
#define VE_VIRUSNAME "CLAM_VIRUSEVENT_VIRUSNAME"
103103

104104
#define FILENAME_DISABLED_MESSAGE "The filename format character has been disabled due to security concerns, use the 'CLAM_VIRUSEVENT_FILENAME' environment variable instead."
105+
#define VIRUSNAME_DISABLED_MESSAGE "The virus name format character has been disabled due to security concerns, use the 'CLAM_VIRUSEVENT_VIRUSNAME' environment variable instead."
105106

106107
void virusaction(const char *filename, const char *virname,
107108
const struct optstruct *opts)
@@ -147,7 +148,7 @@ void virusaction(const char *filename, const char *virname,
147148
}
148149
len = strlen(opt->strarg);
149150
buffer_cmd =
150-
(char *)calloc(len + v * strlen(virname) + f * strlen(FILENAME_DISABLED_MESSAGE) + 1, sizeof(char));
151+
(char *)calloc(len + v * strlen(VIRUSNAME_DISABLED_MESSAGE) + f * strlen(FILENAME_DISABLED_MESSAGE) + 1, sizeof(char));
151152
if (!buffer_cmd) {
152153
if (path)
153154
xfree(env[0]);
@@ -158,8 +159,8 @@ void virusaction(const char *filename, const char *virname,
158159
}
159160
for (i = 0, j = 0; i < len; i++) {
160161
if (i + 1 < len && opt->strarg[i] == '%' && opt->strarg[i + 1] == 'v') {
161-
strcat(buffer_cmd, virname);
162-
j += strlen(virname);
162+
strcat(buffer_cmd, VIRUSNAME_DISABLED_MESSAGE);
163+
j += strlen(VIRUSNAME_DISABLED_MESSAGE);
163164
i++;
164165
} else if (i + 1 < len && opt->strarg[i] == '%' && opt->strarg[i + 1] == 'f') {
165166
strcat(buffer_cmd, FILENAME_DISABLED_MESSAGE);

common/optparser.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ const struct clam_option __clam_options[] = {
369369

370370
{"DisableCache", "disable-cache", 0, CLOPT_TYPE_BOOL, MATCH_BOOL, 0, NULL, 0, OPT_CLAMD | OPT_CLAMSCAN, "This option allows you to disable clamd's caching feature.", "no"},
371371

372-
{"VirusEvent", NULL, 0, CLOPT_TYPE_STRING, NULL, -1, NULL, 0, OPT_CLAMD, "Execute a command when virus is found.\nUse the following environment variables to identify the file and virus names:\n- $CLAM_VIRUSEVENT_FILENAME\n- $CLAM_VIRUSEVENT_VIRUSNAME\nIn the command string, '%v' will also be replaced with the virus name.\nNote: The '%f' filename format character has been disabled and will no longer\nbe replaced with the file name, due to command injection security concerns.\nUse the 'CLAM_VIRUSEVENT_FILENAME' environment variable instead.\nFor the same reason, you should NOT use the environment variables in the\ncommand directly, but should use it carefully from your executed script.", "/opt/send_virus_alert_sms.sh"},
372+
{"VirusEvent", NULL, 0, CLOPT_TYPE_STRING, NULL, -1, NULL, 0, OPT_CLAMD, "Execute a command when virus is found.\nUse the following environment variables to identify the file and virus names:\n- $CLAM_VIRUSEVENT_FILENAME\n- $CLAM_VIRUSEVENT_VIRUSNAME\nNote: The '%v' virus name and '%f' filename format characters have been\ndisabled and will no longer be replaced with the virus or file name, due to\ncommand injection security concerns. Use the environment variables listed\nabove instead.\nFor the same reason, you should NOT use the environment variables directly\nin the command, but should use them carefully from your executed script.", "/opt/send_virus_alert_sms.sh"},
373373

374374
{"ExitOnOOM", NULL, 0, CLOPT_TYPE_BOOL, MATCH_BOOL, 0, NULL, 0, OPT_CLAMD, "Stop the daemon when libclamav reports an out of memory condition.", "yes"},
375375

docs/man/clamd.conf.5.in

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -294,12 +294,12 @@ Execute a command when virus is found.
294294
Use the following environment variables to identify the file and virus names:
295295
- $CLAM_VIRUSEVENT_FILENAME
296296
- $CLAM_VIRUSEVENT_VIRUSNAME
297-
In the command string, '%v' will also be replaced with the virus name.
298-
Note: The '%f' filename format character has been disabled and will no longer
299-
be replaced with the file name, due to command injection security concerns.
300-
Use the 'CLAM_VIRUSEVENT_FILENAME' environment variable instead.
301-
For the same reason, you should NOT use the environment variables in the
302-
command directly, but should use it carefully from your executed script.
297+
Note: The '%v' virus name and '%f' filename format characters have been
298+
disabled and will no longer be replaced with the virus or file name, due to
299+
command injection security concerns. Use the environment variables listed
300+
above instead.
301+
For the same reason, you should NOT use the environment variables directly
302+
in the command, but should use them carefully from your executed script.
303303
\fR
304304
.br
305305
Default: disabled

etc/clamd.conf.sample

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -260,12 +260,12 @@ Example
260260
# Use the following environment variables to identify the file and virus names:
261261
# - $CLAM_VIRUSEVENT_FILENAME
262262
# - $CLAM_VIRUSEVENT_VIRUSNAME
263-
# In the command string, '%v' will also be replaced with the virus name.
264-
# Note: The '%f' filename format character has been disabled and will no longer
265-
# be replaced with the file name, due to command injection security concerns.
266-
# Use the 'CLAM_VIRUSEVENT_FILENAME' environment variable instead.
267-
# For the same reason, you should NOT use the environment variables in the
268-
# command directly, but should use it carefully from your executed script.
263+
# Note: The '%v' virus name and '%f' filename format characters have been
264+
# disabled and will no longer be replaced with the virus or file name, due to
265+
# command injection security concerns. Use the environment variables listed
266+
# above instead.
267+
# For the same reason, you should NOT use the environment variables directly
268+
# in the command, but should use them carefully from your executed script.
269269
# Default: no
270270
#VirusEvent /opt/send_virus_alert_sms.sh
271271

unit_tests/clamd_test.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,7 @@ def test_clamd_08_VirusEvent(self):
675675
self.log.info('verifying log output from virusaction-test.sh: {}'.format(str(TC.path_tmp / 'test-clamd.log')))
676676
self.verify_log(str(TC.path_tmp / 'test-clamd.log'),
677677
expected=['Virus found: ClamAV-Test-File.UNOFFICIAL'],
678-
unexpected=['VirusEvent incorrect', 'VirusName incorrect'])
678+
unexpected=['VirusEvent incorrect', 'VirusName incorrect', 'VirusName argument incorrect'])
679679

680680
def test_clamd_09_clamdscan_ExcludePath(self):
681681
'''
Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
#!/bin/sh
2-
if test ! `basename $CLAM_VIRUSEVENT_FILENAME` = "clam.exe"; then
3-
echo "VirusEvent incorrect: $CLAM_VIRUSEVENT_FILENAME" >$1/test-clamd.log
2+
if test ! "$(basename "$CLAM_VIRUSEVENT_FILENAME")" = "clam.exe"; then
3+
echo "VirusEvent incorrect: $CLAM_VIRUSEVENT_FILENAME" >"$1/test-clamd.log"
44
exit 1
55
fi
66
if test ! "x$CLAM_VIRUSEVENT_VIRUSNAME" = "xClamAV-Test-File.UNOFFICIAL"; then
7-
echo "VirusName incorrect: $CLAM_VIRUSEVENT_VIRUSNAME" >$1/test-clamd.log
7+
echo "VirusName incorrect: $CLAM_VIRUSEVENT_VIRUSNAME" >"$1/test-clamd.log"
88
exit 2
99
fi
10-
echo $2 >$1/test-clamd.log
10+
if test ! "x$2" = "xVirus found: The virus name format character has been disabled due to security concerns, use the 'CLAM_VIRUSEVENT_VIRUSNAME' environment variable instead."; then
11+
echo "VirusName argument incorrect: $2" >"$1/test-clamd.log"
12+
exit 3
13+
fi
14+
echo "Virus found: $CLAM_VIRUSEVENT_VIRUSNAME" >"$1/test-clamd.log"

0 commit comments

Comments
 (0)