If you set an ExcludePath regex in clamd.conf and then perform a
ClamDScan scan with --fdpass --multiscan, it will segfault.
The same issue also affects --fdpass --multiscan scans when using
ExcludePath when scanning a patch that doesn't exist.
The issue is that the filepath isn't being passed along for the path
exclusion regex match, resulting in a NULL deref.
This commit also fixes a possible memory leak if by duplicating the path
for the handle_entry() call _after_ the callback() runs, in case ret
isn't CL_SUCCESS and the function exits without every using the entry
structure or free'ing the copied filename.
The above work temporarily caused a test failure in check_clamd and a
valgrind failure in clamd for the nonexistent file test due to a minor
memory leak. This made it apparent that there were a few other nearby
possible memory leaks.
This commit fixes the above plus cleans up the error handling in clamd's
the file tree walk functions.
| ... | ... |
@@ -147,8 +147,8 @@ cl_error_t scan_callback(STATBUF *sb, char *filename, const char *msg, enum cli_ |
| 147 | 147 |
|
| 148 | 148 |
if (NULL != filename) {
|
| 149 | 149 |
if (CL_SUCCESS != cli_realpath((const char *)filename, &real_filename)) {
|
| 150 |
- conn_reply_errno(scandata->conn, msg, "Failed to determine real path:"); |
|
| 151 |
- logg("^Failed to determine real path for: %s\n", filename);
|
|
| 150 |
+ conn_reply_errno(scandata->conn, msg, "File path check failure:"); |
|
| 151 |
+ logg("^File path check failure for: %s\n", filename);
|
|
| 152 | 152 |
logg("*Quarantine of the file may fail if file path contains symlinks.\n");
|
| 153 | 153 |
} else {
|
| 154 | 154 |
free(filename); |
| ... | ... |
@@ -181,25 +181,30 @@ cl_error_t scan_callback(STATBUF *sb, char *filename, const char *msg, enum cli_ |
| 181 | 181 |
else |
| 182 | 182 |
logg("!Memory allocation failed during cli_ftw()\n");
|
| 183 | 183 |
scandata->errors++; |
| 184 |
+ free(filename); |
|
| 184 | 185 |
return CL_EMEM; |
| 185 | 186 |
case error_stat: |
| 186 |
- conn_reply_errno(scandata->conn, msg, "lstat() failed:"); |
|
| 187 |
- logg("^lstat() failed on: %s\n", msg);
|
|
| 187 |
+ conn_reply_errno(scandata->conn, msg, "File path check failure:"); |
|
| 188 |
+ logg("^File path check failure on: %s\n", msg);
|
|
| 188 | 189 |
scandata->errors++; |
| 190 |
+ free(filename); |
|
| 189 | 191 |
return CL_SUCCESS; |
| 190 | 192 |
case warning_skipped_dir: |
| 191 |
- logg("^Directory recursion limit reached, skipping %s\n",
|
|
| 192 |
- msg); |
|
| 193 |
+ logg("^Directory recursion limit reached, skipping %s\n", msg);
|
|
| 194 |
+ free(filename); |
|
| 193 | 195 |
return CL_SUCCESS; |
| 194 | 196 |
case warning_skipped_link: |
| 195 | 197 |
logg("$Skipping symlink: %s\n", msg);
|
| 198 |
+ free(filename); |
|
| 196 | 199 |
return CL_SUCCESS; |
| 197 | 200 |
case warning_skipped_special: |
| 198 | 201 |
if (msg == scandata->toplevel_path) |
| 199 | 202 |
conn_reply(scandata->conn, msg, "Not supported file type", "ERROR"); |
| 200 | 203 |
logg("*Not supported file type: %s\n", msg);
|
| 204 |
+ free(filename); |
|
| 201 | 205 |
return CL_SUCCESS; |
| 202 | 206 |
case visit_directory_toplev: |
| 207 |
+ free(filename); |
|
| 203 | 208 |
return CL_SUCCESS; |
| 204 | 209 |
case visit_file: |
| 205 | 210 |
break; |
| ... | ... |
@@ -951,7 +951,7 @@ typedef int (*cli_ftw_pathchk)(const char *path, struct cli_ftw_cbdata *data); |
| 951 | 951 |
* which one it is. |
| 952 | 952 |
* If it is a file, it simply calls the callback once, otherwise recurses. |
| 953 | 953 |
*/ |
| 954 |
-int cli_ftw(char *base, int flags, int maxdepth, cli_ftw_cb callback, struct cli_ftw_cbdata *data, cli_ftw_pathchk pathchk); |
|
| 954 |
+cl_error_t cli_ftw(char *base, int flags, int maxdepth, cli_ftw_cb callback, struct cli_ftw_cbdata *data, cli_ftw_pathchk pathchk); |
|
| 955 | 955 |
|
| 956 | 956 |
const char *cli_strerror(int errnum, char *buf, size_t len); |
| 957 | 957 |
|
| ... | ... |
@@ -574,28 +574,46 @@ static int get_filetype(const char *fname, int flags, int need_stat, |
| 574 | 574 |
return stated; |
| 575 | 575 |
} |
| 576 | 576 |
|
| 577 |
-static int handle_filetype(const char *fname, int flags, |
|
| 578 |
- STATBUF *statbuf, int *stated, enum filetype *ft, |
|
| 579 |
- cli_ftw_cb callback, struct cli_ftw_cbdata *data) |
|
| 577 |
+static cl_error_t handle_filetype(const char *fname, int flags, |
|
| 578 |
+ STATBUF *statbuf, int *stated, enum filetype *ft, |
|
| 579 |
+ cli_ftw_cb callback, struct cli_ftw_cbdata *data) |
|
| 580 | 580 |
{
|
| 581 |
- int ret; |
|
| 581 |
+ cl_error_t status = CL_EMEM; |
|
| 582 | 582 |
|
| 583 | 583 |
*stated = get_filetype(fname, flags, flags & CLI_FTW_NEED_STAT, statbuf, ft); |
| 584 | 584 |
|
| 585 | 585 |
if (*stated == -1) {
|
| 586 |
- /* we failed a stat() or lstat() */ |
|
| 587 |
- ret = callback(NULL, NULL, fname, error_stat, data); |
|
| 588 |
- if (ret != CL_SUCCESS) |
|
| 589 |
- return ret; |
|
| 586 |
+ /* we failed a stat() or lstat() */ |
|
| 587 |
+ char *fname_copy = cli_strdup(fname); |
|
| 588 |
+ if (NULL == fname_copy) {
|
|
| 589 |
+ goto done; |
|
| 590 |
+ } |
|
| 591 |
+ |
|
| 592 |
+ status = callback(NULL, fname_copy, fname, error_stat, data); |
|
| 593 |
+ if (status != CL_SUCCESS) {
|
|
| 594 |
+ goto done; |
|
| 595 |
+ } |
|
| 590 | 596 |
*ft = ft_unknown; |
| 591 | 597 |
} else if (*ft == ft_skipped_link || *ft == ft_skipped_special) {
|
| 592 | 598 |
/* skipped filetype */ |
| 593 |
- ret = callback(stated ? statbuf : NULL, NULL, fname, |
|
| 594 |
- *ft == ft_skipped_link ? warning_skipped_link : warning_skipped_special, data); |
|
| 595 |
- if (ret != CL_SUCCESS) |
|
| 596 |
- return ret; |
|
| 599 |
+ char *fname_copy = cli_strdup(fname); |
|
| 600 |
+ if (NULL == fname_copy) {
|
|
| 601 |
+ goto done; |
|
| 602 |
+ } |
|
| 603 |
+ |
|
| 604 |
+ status = callback(stated ? statbuf : NULL, |
|
| 605 |
+ fname_copy, |
|
| 606 |
+ fname, |
|
| 607 |
+ *ft == ft_skipped_link ? warning_skipped_link : warning_skipped_special, |
|
| 608 |
+ data); |
|
| 609 |
+ if (status != CL_SUCCESS) |
|
| 610 |
+ goto done; |
|
| 597 | 611 |
} |
| 598 |
- return CL_SUCCESS; |
|
| 612 |
+ |
|
| 613 |
+ status = CL_SUCCESS; |
|
| 614 |
+ |
|
| 615 |
+done: |
|
| 616 |
+ return status; |
|
| 599 | 617 |
} |
| 600 | 618 |
|
| 601 | 619 |
static int cli_ftw_dir(const char *dirname, int flags, int maxdepth, cli_ftw_cb callback, struct cli_ftw_cbdata *data, cli_ftw_pathchk pathchk); |
| ... | ... |
@@ -608,13 +626,14 @@ static int handle_entry(struct dirent_data *entry, int flags, int maxdepth, cli_ |
| 608 | 608 |
} |
| 609 | 609 |
} |
| 610 | 610 |
|
| 611 |
-int cli_ftw(char *path, int flags, int maxdepth, cli_ftw_cb callback, struct cli_ftw_cbdata *data, cli_ftw_pathchk pathchk) |
|
| 611 |
+cl_error_t cli_ftw(char *path, int flags, int maxdepth, cli_ftw_cb callback, struct cli_ftw_cbdata *data, cli_ftw_pathchk pathchk) |
|
| 612 | 612 |
{
|
| 613 |
+ cl_error_t status = CL_EMEM; |
|
| 613 | 614 |
STATBUF statbuf; |
| 614 | 615 |
enum filetype ft = ft_unknown; |
| 615 | 616 |
struct dirent_data entry; |
| 616 |
- int stated = 0; |
|
| 617 |
- int ret; |
|
| 617 |
+ int stated = 0; |
|
| 618 |
+ char *path_copy = NULL; |
|
| 618 | 619 |
|
| 619 | 620 |
if (((flags & CLI_FTW_TRIM_SLASHES) || pathchk) && path[0] && path[1]) {
|
| 620 | 621 |
char *pathend; |
| ... | ... |
@@ -627,23 +646,49 @@ int cli_ftw(char *path, int flags, int maxdepth, cli_ftw_cb callback, struct cli |
| 627 | 627 |
while (pathend > path && pathend[-1] == *PATHSEP) --pathend; |
| 628 | 628 |
*pathend = '\0'; |
| 629 | 629 |
} |
| 630 |
- if (pathchk && pathchk(path, data) == 1) |
|
| 631 |
- return CL_SUCCESS; |
|
| 632 |
- ret = handle_filetype(path, flags, &statbuf, &stated, &ft, callback, data); |
|
| 633 |
- if (ret != CL_SUCCESS) |
|
| 634 |
- return ret; |
|
| 635 |
- if (ft_skipped(ft)) |
|
| 636 |
- return CL_SUCCESS; |
|
| 637 |
- entry.statbuf = stated ? &statbuf : NULL; |
|
| 638 |
- entry.is_dir = ft == ft_directory; |
|
| 639 |
- entry.filename = entry.is_dir ? NULL : strdup(path); |
|
| 640 |
- entry.dirname = entry.is_dir ? path : NULL; |
|
| 630 |
+ |
|
| 631 |
+ if (pathchk && pathchk(path, data) == 1) {
|
|
| 632 |
+ status = CL_SUCCESS; |
|
| 633 |
+ goto done; |
|
| 634 |
+ } |
|
| 635 |
+ |
|
| 636 |
+ status = handle_filetype(path, flags, &statbuf, &stated, &ft, callback, data); |
|
| 637 |
+ if (status != CL_SUCCESS) {
|
|
| 638 |
+ goto done; |
|
| 639 |
+ } |
|
| 640 |
+ |
|
| 641 |
+ if (ft_skipped(ft)) {
|
|
| 642 |
+ status = CL_SUCCESS; |
|
| 643 |
+ goto done; |
|
| 644 |
+ } |
|
| 645 |
+ |
|
| 646 |
+ entry.statbuf = stated ? &statbuf : NULL; |
|
| 647 |
+ entry.is_dir = ft == ft_directory; |
|
| 648 |
+ |
|
| 641 | 649 |
if (entry.is_dir) {
|
| 642 |
- ret = callback(entry.statbuf, NULL, path, visit_directory_toplev, data); |
|
| 643 |
- if (ret != CL_SUCCESS) |
|
| 644 |
- return ret; |
|
| 650 |
+ path_copy = cli_strdup(path); |
|
| 651 |
+ if (NULL == path_copy) {
|
|
| 652 |
+ goto done; |
|
| 653 |
+ } |
|
| 654 |
+ |
|
| 655 |
+ status = callback(entry.statbuf, path_copy, path, visit_directory_toplev, data); |
|
| 656 |
+ if (status != CL_SUCCESS) {
|
|
| 657 |
+ goto done; |
|
| 658 |
+ } |
|
| 659 |
+ } |
|
| 660 |
+ |
|
| 661 |
+ path_copy = cli_strdup(path); |
|
| 662 |
+ if (NULL == path_copy) {
|
|
| 663 |
+ goto done; |
|
| 645 | 664 |
} |
| 646 |
- return handle_entry(&entry, flags, maxdepth, callback, data, pathchk); |
|
| 665 |
+ |
|
| 666 |
+ entry.filename = entry.is_dir ? NULL : path_copy; |
|
| 667 |
+ entry.dirname = entry.is_dir ? path : NULL; |
|
| 668 |
+ |
|
| 669 |
+ status = handle_entry(&entry, flags, maxdepth, callback, data, pathchk); |
|
| 670 |
+ |
|
| 671 |
+done: |
|
| 672 |
+ return status; |
|
| 647 | 673 |
} |
| 648 | 674 |
|
| 649 | 675 |
static int cli_ftw_dir(const char *dirname, int flags, int maxdepth, cli_ftw_cb callback, struct cli_ftw_cbdata *data, cli_ftw_pathchk pathchk) |
| ... | ... |
@@ -146,7 +146,7 @@ static void conn_teardown(void) |
| 146 | 146 |
|
| 147 | 147 |
#define NONEXISTENT PATHSEP "nonexistent\vfilename" |
| 148 | 148 |
|
| 149 |
-#define NONEXISTENT_REPLY NONEXISTENT ": lstat() failed: No such file or directory. ERROR" |
|
| 149 |
+#define NONEXISTENT_REPLY NONEXISTENT ": File path check failure: No such file or directory. ERROR" |
|
| 150 | 150 |
|
| 151 | 151 |
#ifndef _WIN32 |
| 152 | 152 |
#define ACCDENIED OBJDIR PATHSEP "accdenied" |