From 12a516c237022aca793585421b996b7b81d1fd30 Mon Sep 17 00:00:00 2001 From: "Stiliyan Tonev (Bark)" Date: Mon, 29 Jul 2024 13:18:19 +0300 Subject: [PATCH 01/10] fix: Fix failure to start when log file is missing. --- common/output.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/common/output.c b/common/output.c index 72e57ea96e..e6d949e2e9 100644 --- a/common/output.c +++ b/common/output.c @@ -271,10 +271,12 @@ static int logg_open(void) if (logg_file) if (logg_size > 0) - if (CLAMSTAT(logg_file, &sb) != -1) + if (CLAMSTAT(logg_file, &sb) != -1) { if (sb.st_size > logg_size) if (rename_logg(&sb)) return -1; + } else + return -1; return 0; } @@ -335,7 +337,37 @@ int logg(loglevel_t loglevel, const char *str, ...) pthread_mutex_lock(&logg_mutex); #endif - logg_open(); + if(logg_open() == -1) { + printf("WARNING: Can't open %s in append mode (check permissions!). Will attempt to create it.\n", logg_file); + char* current_dir = "/"; + char* file_path = strdup(logg_file); + char* token = strtok(file_path, "/"); + current_dir = (char*)malloc(2); + strcpy(current_dir, "/"); + STATBUF sb; + + while (token != NULL) { + current_dir = (char*)realloc(current_dir, strlen(current_dir) + strlen(token) + 2); + strcat(current_dir, token); + token = strtok(NULL, "/"); + if(LSTAT(current_dir, &sb) == -1) { + if(mkdir(current_dir, 0755) == -1) { + printf("ERROR: Failed to create required directory %s. Will continue without writing in %s.\n", current_dir, logg_file); + free(current_dir); + free(file_path); + return -1; + } + } + strcat(current_dir, "/"); + } + //create file + if ((logg_fp = fopen(logg_file, "at")) == NULL) { + printf("ERROR: Can't open %s in append mode (check permissions!).\n", logg_file); + free(current_dir); + free(file_path); + return -1; + } + } if (!logg_fp && logg_file) { old_umask = umask(0037); From 76a31a261a684c005926fb46db4615beb8a7a8a9 Mon Sep 17 00:00:00 2001 From: "Stiliyan Tonev (Bark)" Date: Mon, 29 Jul 2024 14:12:28 +0300 Subject: [PATCH 02/10] Escape after the last slash, otherwise a directory will be created while we want a file. --- common/output.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/common/output.c b/common/output.c index e6d949e2e9..7d07cc9b84 100644 --- a/common/output.c +++ b/common/output.c @@ -350,6 +350,9 @@ int logg(loglevel_t loglevel, const char *str, ...) current_dir = (char*)realloc(current_dir, strlen(current_dir) + strlen(token) + 2); strcat(current_dir, token); token = strtok(NULL, "/"); + if(token == NULL) { + break; + } if(LSTAT(current_dir, &sb) == -1) { if(mkdir(current_dir, 0755) == -1) { printf("ERROR: Failed to create required directory %s. Will continue without writing in %s.\n", current_dir, logg_file); @@ -360,7 +363,6 @@ int logg(loglevel_t loglevel, const char *str, ...) } strcat(current_dir, "/"); } - //create file if ((logg_fp = fopen(logg_file, "at")) == NULL) { printf("ERROR: Can't open %s in append mode (check permissions!).\n", logg_file); free(current_dir); From c6c8d1fe582c7c8df1a2710b75b0ece3d1b46401 Mon Sep 17 00:00:00 2001 From: "Stiliyan Tonev (Bark)" Date: Mon, 29 Jul 2024 17:15:55 +0300 Subject: [PATCH 03/10] fix: Allow folder creation as root and give access to clamav. --- common/output.c | 39 +++------------------------ freshclam/freshclam.c | 42 +++++++++++++++-------------- libfreshclam/libfreshclam.c | 54 +++++++++++++++++++++++++++++++++++++ libfreshclam/libfreshclam.h | 1 + 4 files changed, 81 insertions(+), 55 deletions(-) diff --git a/common/output.c b/common/output.c index 7d07cc9b84..82bf0ed5df 100644 --- a/common/output.c +++ b/common/output.c @@ -55,7 +55,9 @@ #include "clamav.h" #include "others.h" #include "str.h" - +#include "output.h" +#include "optparser.h" +#include #include "output.h" #ifdef CL_THREAD_SAFE @@ -336,40 +338,7 @@ int logg(loglevel_t loglevel, const char *str, ...) #ifdef CL_THREAD_SAFE pthread_mutex_lock(&logg_mutex); #endif - - if(logg_open() == -1) { - printf("WARNING: Can't open %s in append mode (check permissions!). Will attempt to create it.\n", logg_file); - char* current_dir = "/"; - char* file_path = strdup(logg_file); - char* token = strtok(file_path, "/"); - current_dir = (char*)malloc(2); - strcpy(current_dir, "/"); - STATBUF sb; - - while (token != NULL) { - current_dir = (char*)realloc(current_dir, strlen(current_dir) + strlen(token) + 2); - strcat(current_dir, token); - token = strtok(NULL, "/"); - if(token == NULL) { - break; - } - if(LSTAT(current_dir, &sb) == -1) { - if(mkdir(current_dir, 0755) == -1) { - printf("ERROR: Failed to create required directory %s. Will continue without writing in %s.\n", current_dir, logg_file); - free(current_dir); - free(file_path); - return -1; - } - } - strcat(current_dir, "/"); - } - if ((logg_fp = fopen(logg_file, "at")) == NULL) { - printf("ERROR: Can't open %s in append mode (check permissions!).\n", logg_file); - free(current_dir); - free(file_path); - return -1; - } - } + logg_open(); if (!logg_fp && logg_file) { old_umask = umask(0037); diff --git a/freshclam/freshclam.c b/freshclam/freshclam.c index 8953024c82..6d4fdd5df4 100644 --- a/freshclam/freshclam.c +++ b/freshclam/freshclam.c @@ -811,26 +811,6 @@ static fc_error_t initialize(struct optstruct *opts) #endif } -#ifdef HAVE_PWD_H - /* Drop database privileges here if we are not planning on daemonizing. If - * we are, we should wait until after we create the PidFile to drop - * privileges. That way, it is owned by root (or whoever started freshclam), - * and no one can change it. */ - if (!optget(opts, "daemon")->enabled) { - /* - * freshclam shouldn't work with root privileges. - * Drop privileges to the DatabaseOwner user, if specified. - * Pass NULL for the log file name, because it hasn't been created yet. - */ - ret = drop_privileges(optget(opts, "DatabaseOwner")->strarg, NULL); - if (ret) { - logg(LOGG_ERROR, "Failed to switch to %s user.\n", optget(opts, "DatabaseOwner")->strarg); - status = FC_ECONFIG; - goto done; - } - } -#endif /* HAVE_PWD_H */ - /* * Initialize libclamav. */ @@ -982,6 +962,7 @@ static fc_error_t initialize(struct optstruct *opts) fcConfig.requestTimeout = optget(opts, "ReceiveTimeout")->numarg; fcConfig.bCompressLocalDatabase = optget(opts, "CompressLocalDatabase")->enabled; + fcConfig.dbOwner = optget(opts, "DatabaseOwner")->strarg; /* * Initialize libfreshclam. @@ -992,6 +973,27 @@ static fc_error_t initialize(struct optstruct *opts) goto done; } +#ifdef HAVE_PWD_H + /* Drop database privileges here (cause of log file creation in /var/log) if we are not planning on daemonizing. If + * we are, we should wait until after we create the PidFile to drop + * privileges. That way, it is owned by root (or whoever started freshclam), + * and no one can change it. */ + if (!optget(opts, "daemon")->enabled) { + /* + * freshclam shouldn't work with root privileges. + * Drop privileges to the DatabaseOwner user, if specified. + * Pass NULL for the log file name, because it hasn't been created yet. + */ + ret = drop_privileges(optget(opts, "DatabaseOwner")->strarg, NULL); + if (ret) { + logg(LOGG_ERROR, "Failed to switch to %s user.\n", optget(opts, "DatabaseOwner")->strarg); + status = FC_ECONFIG; + goto done; + } + } +#endif /* HAVE_PWD_H */ + + /* * Set libfreshclam callback functions. */ diff --git a/libfreshclam/libfreshclam.c b/libfreshclam/libfreshclam.c index 400f44694c..c541c97e0e 100644 --- a/libfreshclam/libfreshclam.c +++ b/libfreshclam/libfreshclam.c @@ -123,6 +123,58 @@ const char *fc_strerror(fc_error_t fcerror) } } +int fc_upsert_logg_file(fc_config *fcConfig) +{ + int ret = 0; + char* current_dir = "/"; + char* file_path = strdup(fcConfig->logFile); + char* log_file = fcConfig->logFile; + char* token = strtok(file_path, "/"); + FILE *logg_fp = NULL; + struct passwd *current_user = getpwuid(getuid()); + struct passwd *db_owner = getpwnam(fcConfig->dbOwner); + current_dir = (char*)malloc(2); + strcpy(current_dir, "/"); + STATBUF sb; + + while (token != NULL) { + current_dir = (char*)realloc(current_dir, strlen(current_dir) + strlen(token) + 2); + strcat(current_dir, token); + token = strtok(NULL, "/"); + if(token == NULL) { + break; + } + if(LSTAT(current_dir, &sb) == -1) { + if(mkdir(current_dir, 0755) == -1) { + printf("ERROR: Failed to create required directory %s. Will continue without writing in %s.\n", current_dir, log_file); + ret = -1; + goto cleanup; + } + if(chown(current_dir, db_owner->pw_uid, db_owner->pw_gid) == -1) { + printf("ERROR: Failed to change owner of %s to %s. Will continue without writing in %s.\n", current_dir, fcConfig->dbOwner, log_file); + ret = -1; + goto cleanup; + } + } + strcat(current_dir, "/"); + } + if ((logg_fp = fopen(log_file, "at")) == NULL) { + printf("ERROR: Can't open %s in append mode (check permissions!).\n", log_file); + ret = -1; + goto cleanup; + } + lchown(log_file, db_owner->pw_uid, db_owner->pw_gid); + +cleanup: + free(current_dir); + free(file_path); + if(logg_fp != NULL) { + fclose(logg_fp); + } + + return ret; +} + fc_error_t fc_initialize(fc_config *fcConfig) { fc_error_t status = FC_EARG; @@ -157,6 +209,8 @@ fc_error_t fc_initialize(fc_config *fcConfig) logg_rotate = (fcConfig->logFlags & FC_CONFIG_LOG_ROTATE) ? 1 : 0; logg_size = fcConfig->maxLogSize; /* Set a log file if requested, and is not already set */ + fc_upsert_logg_file(fcConfig); + if ((NULL == logg_file) && (NULL != fcConfig->logFile)) { logg_file = cli_safer_strdup(fcConfig->logFile); if (0 != logg(LOGG_INFO_NF, "--------------------------------------\n")) { diff --git a/libfreshclam/libfreshclam.h b/libfreshclam/libfreshclam.h index 25d7293fef..36bfa235c7 100644 --- a/libfreshclam/libfreshclam.h +++ b/libfreshclam/libfreshclam.h @@ -60,6 +60,7 @@ typedef struct fc_config_ { const char *proxyPassword; /**< (optional) Password for proxy server authentication. */ const char *databaseDirectory; /**< Filepath of database directory. */ const char *tempDirectory; /**< Filepath to store temp files. */ + const char *dbOwner; /**< Owner of DB, used when creating log file/folder. */ } fc_config; typedef enum fc_error_tag { From eee394968f773034ff7d13af139a0b22f6b3d599 Mon Sep 17 00:00:00 2001 From: "Stiliyan Tonev (Bark)" Date: Mon, 29 Jul 2024 17:17:48 +0300 Subject: [PATCH 04/10] revert changes done during testing. --- common/output.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/common/output.c b/common/output.c index 82bf0ed5df..a2bfea37c0 100644 --- a/common/output.c +++ b/common/output.c @@ -56,9 +56,6 @@ #include "others.h" #include "str.h" #include "output.h" -#include "optparser.h" -#include -#include "output.h" #ifdef CL_THREAD_SAFE #include @@ -273,12 +270,10 @@ static int logg_open(void) if (logg_file) if (logg_size > 0) - if (CLAMSTAT(logg_file, &sb) != -1) { + if (CLAMSTAT(logg_file, &sb) != -1) if (sb.st_size > logg_size) if (rename_logg(&sb)) return -1; - } else - return -1; return 0; } @@ -338,6 +333,7 @@ int logg(loglevel_t loglevel, const char *str, ...) #ifdef CL_THREAD_SAFE pthread_mutex_lock(&logg_mutex); #endif + logg_open(); if (!logg_fp && logg_file) { From aa3480d059a76247d31480222cf29851af41585d Mon Sep 17 00:00:00 2001 From: "Stiliyan Tonev (Bark)" Date: Mon, 29 Jul 2024 17:18:40 +0300 Subject: [PATCH 05/10] return removed empty line. --- common/output.c | 1 + 1 file changed, 1 insertion(+) diff --git a/common/output.c b/common/output.c index a2bfea37c0..72e57ea96e 100644 --- a/common/output.c +++ b/common/output.c @@ -55,6 +55,7 @@ #include "clamav.h" #include "others.h" #include "str.h" + #include "output.h" #ifdef CL_THREAD_SAFE From 4815aef6b4bf6446455af661fd0a852cc120b060 Mon Sep 17 00:00:00 2001 From: "Stiliyan Tonev (Bark)" Date: Thu, 15 Aug 2024 09:00:21 +0300 Subject: [PATCH 06/10] ref: Formating and cleanup --- freshclam/freshclam.c | 1 - libfreshclam/libfreshclam.c | 51 ++++++++++++++++++------------------- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/freshclam/freshclam.c b/freshclam/freshclam.c index 6d4fdd5df4..fd697b9d50 100644 --- a/freshclam/freshclam.c +++ b/freshclam/freshclam.c @@ -993,7 +993,6 @@ static fc_error_t initialize(struct optstruct *opts) } #endif /* HAVE_PWD_H */ - /* * Set libfreshclam callback functions. */ diff --git a/libfreshclam/libfreshclam.c b/libfreshclam/libfreshclam.c index c541c97e0e..b8d410c537 100644 --- a/libfreshclam/libfreshclam.c +++ b/libfreshclam/libfreshclam.c @@ -126,49 +126,48 @@ const char *fc_strerror(fc_error_t fcerror) int fc_upsert_logg_file(fc_config *fcConfig) { int ret = 0; - char* current_dir = "/"; - char* file_path = strdup(fcConfig->logFile); - char* log_file = fcConfig->logFile; - char* token = strtok(file_path, "/"); - FILE *logg_fp = NULL; - struct passwd *current_user = getpwuid(getuid()); - struct passwd *db_owner = getpwnam(fcConfig->dbOwner); - current_dir = (char*)malloc(2); - strcpy(current_dir, "/"); + char *current_path, *file_path = strdup(fcConfig->logFile), *token; + FILE *logg_fp = NULL; + token = strtok(file_path, PATHSEP); + struct passwd *current_user = getpwuid(getuid()), *db_owner = getpwnam(fcConfig->dbOwner); + current_path = (char *)malloc(2); + strcpy(current_path, PATHSEP); STATBUF sb; while (token != NULL) { - current_dir = (char*)realloc(current_dir, strlen(current_dir) + strlen(token) + 2); - strcat(current_dir, token); - token = strtok(NULL, "/"); - if(token == NULL) { + current_path = (char *)realloc(current_path, strlen(current_path) + strlen(token) + 2); + strcat(current_path, token); + token = strtok(NULL, PATHSEP); + if (token == NULL) { break; } - if(LSTAT(current_dir, &sb) == -1) { - if(mkdir(current_dir, 0755) == -1) { - printf("ERROR: Failed to create required directory %s. Will continue without writing in %s.\n", current_dir, log_file); + if (LSTAT(current_path, &sb) == -1) { + if (mkdir(current_path, 0755) == -1) { + printf("ERROR: Failed to create required directory %s. Will continue without writing in %s.\n", current_path, fcConfig->logFile); ret = -1; goto cleanup; } - if(chown(current_dir, db_owner->pw_uid, db_owner->pw_gid) == -1) { - printf("ERROR: Failed to change owner of %s to %s. Will continue without writing in %s.\n", current_dir, fcConfig->dbOwner, log_file); - ret = -1; - goto cleanup; + if (current_user->pw_uid != db_owner->pw_uid) { + if (chown(current_path, db_owner->pw_uid, db_owner->pw_gid) == -1) { + printf("ERROR: Failed to change owner of %s to %s. Will continue without writing in %s.\n", current_path, fcConfig->dbOwner, fcConfig->logFile); + ret = -1; + goto cleanup; + } } } - strcat(current_dir, "/"); + strcat(current_path, PATHSEP); } - if ((logg_fp = fopen(log_file, "at")) == NULL) { - printf("ERROR: Can't open %s in append mode (check permissions!).\n", log_file); + if ((logg_fp = fopen(fcConfig->logFile, "at")) == NULL) { + printf("ERROR: Can't open %s in append mode (check permissions!).\n", fcConfig->logFile); ret = -1; goto cleanup; } - lchown(log_file, db_owner->pw_uid, db_owner->pw_gid); + lchown(fcConfig->logFile, db_owner->pw_uid, db_owner->pw_gid); cleanup: - free(current_dir); + free(current_path); free(file_path); - if(logg_fp != NULL) { + if (logg_fp != NULL) { fclose(logg_fp); } From 94f1871e01f711c048a490a9f139da1e34418e43 Mon Sep 17 00:00:00 2001 From: "Stiliyan Tonev (Bark)" Date: Fri, 30 Aug 2024 10:55:45 +0300 Subject: [PATCH 07/10] Use `cli_safer_strdup` and `cli_strtok` instead of the vanila functions. --- libfreshclam/libfreshclam.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/libfreshclam/libfreshclam.c b/libfreshclam/libfreshclam.c index b8d410c537..49b9819e60 100644 --- a/libfreshclam/libfreshclam.c +++ b/libfreshclam/libfreshclam.c @@ -125,19 +125,19 @@ const char *fc_strerror(fc_error_t fcerror) int fc_upsert_logg_file(fc_config *fcConfig) { - int ret = 0; - char *current_path, *file_path = strdup(fcConfig->logFile), *token; + int ret = 0, field_no = 1; + char *current_path, *file_path = cli_safer_strdup(fcConfig->logFile), *token; FILE *logg_fp = NULL; - token = strtok(file_path, PATHSEP); + token = cli_strtok(file_path, field_no++, PATHSEP); struct passwd *current_user = getpwuid(getuid()), *db_owner = getpwnam(fcConfig->dbOwner); current_path = (char *)malloc(2); strcpy(current_path, PATHSEP); STATBUF sb; - while (token != NULL) { current_path = (char *)realloc(current_path, strlen(current_path) + strlen(token) + 2); strcat(current_path, token); - token = strtok(NULL, PATHSEP); + free(token); + token = cli_strtok(file_path, field_no++, PATHSEP); if (token == NULL) { break; } @@ -148,7 +148,7 @@ int fc_upsert_logg_file(fc_config *fcConfig) goto cleanup; } if (current_user->pw_uid != db_owner->pw_uid) { - if (chown(current_path, db_owner->pw_uid, db_owner->pw_gid) == -1) { + if (lchown(current_path, db_owner->pw_uid, db_owner->pw_gid) == -1) { printf("ERROR: Failed to change owner of %s to %s. Will continue without writing in %s.\n", current_path, fcConfig->dbOwner, fcConfig->logFile); ret = -1; goto cleanup; @@ -165,6 +165,9 @@ int fc_upsert_logg_file(fc_config *fcConfig) lchown(fcConfig->logFile, db_owner->pw_uid, db_owner->pw_gid); cleanup: + if (token != NULL) { + free(token); + } free(current_path); free(file_path); if (logg_fp != NULL) { From e8ba5b23cf2677cb1f9ae407be22304778b5a489 Mon Sep 17 00:00:00 2001 From: "Stiliyan Tonev (Bark)" Date: Fri, 30 Aug 2024 14:07:00 +0300 Subject: [PATCH 08/10] Handle case where log file is not set. --- libfreshclam/libfreshclam.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libfreshclam/libfreshclam.c b/libfreshclam/libfreshclam.c index 49b9819e60..e99257d080 100644 --- a/libfreshclam/libfreshclam.c +++ b/libfreshclam/libfreshclam.c @@ -125,6 +125,9 @@ const char *fc_strerror(fc_error_t fcerror) int fc_upsert_logg_file(fc_config *fcConfig) { + if (fcConfig->logFile == NULL) { + return 0; + } int ret = 0, field_no = 1; char *current_path, *file_path = cli_safer_strdup(fcConfig->logFile), *token; FILE *logg_fp = NULL; From 35e0ffd6189b1f7871884094a245fc071d32cf99 Mon Sep 17 00:00:00 2001 From: "Stiliyan Tonev (Bark)" Date: Fri, 30 Aug 2024 14:54:14 +0300 Subject: [PATCH 09/10] Do not attempt chmod on windows. --- libfreshclam/libfreshclam.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libfreshclam/libfreshclam.c b/libfreshclam/libfreshclam.c index e99257d080..c4f5077638 100644 --- a/libfreshclam/libfreshclam.c +++ b/libfreshclam/libfreshclam.c @@ -150,6 +150,7 @@ int fc_upsert_logg_file(fc_config *fcConfig) ret = -1; goto cleanup; } +#ifndef _WIN32 if (current_user->pw_uid != db_owner->pw_uid) { if (lchown(current_path, db_owner->pw_uid, db_owner->pw_gid) == -1) { printf("ERROR: Failed to change owner of %s to %s. Will continue without writing in %s.\n", current_path, fcConfig->dbOwner, fcConfig->logFile); @@ -157,6 +158,7 @@ int fc_upsert_logg_file(fc_config *fcConfig) goto cleanup; } } +#endif /* _WIN32 */ } strcat(current_path, PATHSEP); } @@ -165,7 +167,9 @@ int fc_upsert_logg_file(fc_config *fcConfig) ret = -1; goto cleanup; } +#ifndef _WIN32 lchown(fcConfig->logFile, db_owner->pw_uid, db_owner->pw_gid); +#endif /* _WIN32 */ cleanup: if (token != NULL) { From 7c7c9d661021c6f9852fba6cb8bc47b60e0f2541 Mon Sep 17 00:00:00 2001 From: "Stiliyan Tonev (Bark)" Date: Fri, 30 Aug 2024 15:17:57 +0300 Subject: [PATCH 10/10] Fix windows build due to getpwuid and getpwname functions --- libfreshclam/libfreshclam.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/libfreshclam/libfreshclam.c b/libfreshclam/libfreshclam.c index c4f5077638..e21589baa3 100644 --- a/libfreshclam/libfreshclam.c +++ b/libfreshclam/libfreshclam.c @@ -130,9 +130,13 @@ int fc_upsert_logg_file(fc_config *fcConfig) } int ret = 0, field_no = 1; char *current_path, *file_path = cli_safer_strdup(fcConfig->logFile), *token; - FILE *logg_fp = NULL; - token = cli_strtok(file_path, field_no++, PATHSEP); - struct passwd *current_user = getpwuid(getuid()), *db_owner = getpwnam(fcConfig->dbOwner); + FILE *logg_fp = NULL; + token = cli_strtok(file_path, field_no++, PATHSEP); + struct passwd *current_user, *db_owner; +#ifndef _WIN32 + current_user = getpwuid(getuid()); + db_owner = getpwnam(fcConfig->dbOwner); +#endif /* _WIN32 */ current_path = (char *)malloc(2); strcpy(current_path, PATHSEP); STATBUF sb;