From 5c6b2d6d4ba5260a877bb8a0e44a719ce388d1b0 Mon Sep 17 00:00:00 2001 From: Micah Snyder Date: Fri, 3 May 2024 10:37:58 -0400 Subject: [PATCH 1/2] For RAR load, check LD_LIBRARY_PATH before checking install path ClamAV initalization's rarload() function tries to load libclamunrar_iface from the install path before checking under LD_LIBRARY_PATH. This means the unit tests will use the wrong unrar library if testing on a system where ClamAV is already installed. In the event there is an ABI break between versions, this will cause a bunch of tests to fail. This commit fixes the issue by checking for libclamunrar_iface under LD_LIBRARY_PATH *first* before checking in the install lib directory. Note in the previous version we were also checking LD_LIBRARY_PATH on Windows, which is not a thing. I removed this. Fixes: https://github.com/Cisco-Talos/clamav/issues/1249 Also removed check for WARN_DLOPEN_FAIL define, which was not used, and mistakenly set for the unrar library build target. --- libclamav/others.c | 205 ++++++++++++++++++------------------ libclamunrar/CMakeLists.txt | 1 - 2 files changed, 101 insertions(+), 105 deletions(-) diff --git a/libclamav/others.c b/libclamav/others.c index 2b9fc6f1c7..79fd2c122a 100644 --- a/libclamav/others.c +++ b/libclamav/others.c @@ -92,106 +92,27 @@ static int is_rar_inited = 0; #define PASTE2(a, b) a #b #define PASTE(a, b) PASTE2(a, b) -static void *load_module(const char *name, const char *featurename) -{ #ifdef _WIN32 - static const char *suffixes[] = {LT_MODULE_EXT}; -#else - static const char *suffixes[] = { - LT_MODULE_EXT "." LIBCLAMAV_FULLVER, - PASTE(LT_MODULE_EXT ".", LIBCLAMAV_MAJORVER), - LT_MODULE_EXT, - "." LT_LIBEXT}; -#endif - const char *searchpath; +static void *load_module(const char *name, const char *featurename) +{ + HMODULE rhandle = NULL; char modulename[128]; size_t i; -#ifdef _WIN32 - HMODULE rhandle = NULL; -#else - void *rhandle; -#endif -#ifdef _WIN32 /* - * First try a standard LoadLibraryA() without specifying a full path. + * For Windows, just try a standard LoadLibraryA() with each of the different possible suffixes. * For more information on the DLL search order, see: - * https://docs.microsoft.com/en-us/windows/desktop/Dlls/dynamic-link-library-search-order + * https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order */ cli_dbgmsg("searching for %s\n", featurename); -#else - /* - * First search using the provided SEARCH_LIBDIR (e.g. "/lib") - * Known issue: If an older clamav version is already installed, the clamav - * unit tests using this function will load the older library version from - * the install path first. - */ - searchpath = SEARCH_LIBDIR; - cli_dbgmsg("searching for %s, user-searchpath: %s\n", featurename, searchpath); -#endif - - for (i = 0; i < sizeof(suffixes) / sizeof(suffixes[0]); i++) { -#ifdef _WIN32 - snprintf(modulename, sizeof(modulename), "%s%s", name, suffixes[i]); - rhandle = LoadLibraryA(modulename); -#else // !_WIN32 - snprintf(modulename, sizeof(modulename), "%s" PATHSEP "%s%s", searchpath, name, suffixes[i]); - rhandle = dlopen(modulename, RTLD_NOW); -#endif // !_WIN32 - if (rhandle) { - break; - } - cli_dbgmsg("searching for %s: %s not found\n", featurename, modulename); - } + snprintf(modulename, sizeof(modulename), "%s%s", name, LT_MODULE_EXT); + rhandle = LoadLibraryA(modulename); if (NULL == rhandle) { - char *ld_library_path = NULL; - /* - * library not found. - * Try again using LD_LIBRARY_PATH environment variable for the path. - */ - ld_library_path = getenv("LD_LIBRARY_PATH"); - if (NULL != ld_library_path) { -#define MAX_LIBRARY_PATHS 10 - size_t token_index; - size_t tokens_count; - const char *tokens[MAX_LIBRARY_PATHS]; - - char *tokenized_library_path = NULL; - - tokenized_library_path = strdup(ld_library_path); - tokens_count = cli_strtokenize(tokenized_library_path, ':', MAX_LIBRARY_PATHS, tokens); - - for (token_index = 0; token_index < tokens_count; token_index++) { - cli_dbgmsg("searching for %s, LD_LIBRARY_PATH: %s\n", featurename, tokens[token_index]); - - for (i = 0; i < sizeof(suffixes) / sizeof(suffixes[0]); i++) { - snprintf(modulename, sizeof(modulename), "%s" PATHSEP "%s%s", tokens[token_index], name, suffixes[i]); -#ifdef _WIN32 - rhandle = LoadLibraryA(modulename); -#else // !_WIN32 - rhandle = dlopen(modulename, RTLD_NOW); -#endif // !_WIN32 - if (rhandle) { - break; - } - - cli_dbgmsg("searching for %s: %s not found\n", featurename, modulename); - } + char *err = NULL; - if (rhandle) { - break; - } - } - free(tokenized_library_path); - } - } - - if (NULL == rhandle) { -#ifdef _WIN32 - char *err = NULL; DWORD lasterr = GetLastError(); if (0 < lasterr) { FormatMessageA( @@ -203,36 +124,112 @@ static void *load_module(const char *name, const char *featurename) 0, NULL); } -#else // !_WIN32 - const char *err = dlerror(); -#endif // !_WIN32 -#ifdef WARN_DLOPEN_FAIL if (NULL == err) { - cli_warnmsg("Cannot dlopen %s: Unknown error - %s support unavailable\n", name, featurename); + cli_dbgmsg("Cannot LoadLibraryA %s: Unknown error - %s support unavailable\n", name, featurename); } else { - cli_warnmsg("Cannot dlopen %s: %s - %s support unavailable\n", name, err, featurename); + cli_dbgmsg("Cannot LoadLibraryA %s: %s - %s support unavailable\n", name, err, featurename); + LocalFree(err); } + + goto done; + } + + cli_dbgmsg("%s support loaded from %s\n", featurename, modulename); + +done: + + return (void *)rhandle; +} + #else - if (NULL == err) { - cli_dbgmsg("Cannot dlopen %s: Unknown error - %s support unavailable\n", name, featurename); - } else { - cli_dbgmsg("Cannot dlopen %s: %s - %s support unavailable\n", name, err, featurename); + +static void *load_module(const char *name, const char *featurename) +{ + static const char *suffixes[] = { + LT_MODULE_EXT "." LIBCLAMAV_FULLVER, + PASTE(LT_MODULE_EXT ".", LIBCLAMAV_MAJORVER), + LT_MODULE_EXT, + "." LT_LIBEXT}; + void *rhandle = NULL; + char *tokenized_library_path = NULL; + char *ld_library_path = NULL; + const char *err; + + char modulename[128]; + size_t i; + + /* + * First try using LD_LIBRARY_PATH environment variable for the path. + * We do this first because LD_LIBRARY_PATH is intended as an option to override the installed library path. + * + * We don't do this for Windows because Windows doesn't have an equivalent to LD_LIBRARY_PATH + * and because LoadLibraryA() will search the executable's folder, which works for the unit tests. + */ + ld_library_path = getenv("LD_LIBRARY_PATH"); + if (NULL != ld_library_path && strlen(ld_library_path) > 0) { +#define MAX_LIBRARY_PATHS 10 + size_t token_index; + size_t tokens_count; + const char *tokens[MAX_LIBRARY_PATHS]; + + /* + * LD_LIBRARY_PATH may be a colon-separated list of directories. + * Tokenize the list and try to load the library from each directory. + */ + tokenized_library_path = strdup(ld_library_path); + tokens_count = cli_strtokenize(tokenized_library_path, ':', MAX_LIBRARY_PATHS, tokens); + + for (token_index = 0; token_index < tokens_count; token_index++) { + cli_dbgmsg("searching for %s, LD_LIBRARY_PATH: %s\n", featurename, tokens[token_index]); + + for (i = 0; i < sizeof(suffixes) / sizeof(suffixes[0]); i++) { + snprintf(modulename, sizeof(modulename), "%s" PATHSEP "%s%s", tokens[token_index], name, suffixes[i]); + + rhandle = dlopen(modulename, RTLD_NOW); + if (NULL != rhandle) { + cli_dbgmsg("%s support loaded from %s\n", featurename, modulename); + goto done; + } + + cli_dbgmsg("searching for %s: %s not found\n", featurename, modulename); + } } -#endif + } -#ifdef _WIN32 - if (NULL != err) { - LocalFree(err); + /* + * Search in "/lib" checking with each of the different possible suffixes. + */ + cli_dbgmsg("searching for %s, user-searchpath: %s\n", featurename, SEARCH_LIBDIR); + + for (i = 0; i < sizeof(suffixes) / sizeof(suffixes[0]); i++) { + snprintf(modulename, sizeof(modulename), "%s" PATHSEP "%s%s", SEARCH_LIBDIR, name, suffixes[i]); + + rhandle = dlopen(modulename, RTLD_NOW); + if (NULL != rhandle) { + cli_dbgmsg("%s support loaded from %s\n", featurename, modulename); + goto done; } -#endif - return rhandle; + + cli_dbgmsg("searching for %s: %s not found\n", featurename, modulename); } - cli_dbgmsg("%s support loaded from %s\n", featurename, modulename); + err = dlerror(); + if (NULL == err) { + cli_dbgmsg("Cannot dlopen %s: Unknown error - %s support unavailable\n", name, featurename); + } else { + cli_dbgmsg("Cannot dlopen %s: %s - %s support unavailable\n", name, err, featurename); + } + +done: + + free(tokenized_library_path); + return (void *)rhandle; } +#endif + #ifdef _WIN32 static void *get_module_function(HMODULE handle, const char *name) diff --git a/libclamunrar/CMakeLists.txt b/libclamunrar/CMakeLists.txt index ffc09c9bd5..2500a42fa9 100644 --- a/libclamunrar/CMakeLists.txt +++ b/libclamunrar/CMakeLists.txt @@ -1,7 +1,6 @@ # Copyright (C) 2019-2024 Cisco Systems, Inc. and/or its affiliates. All rights reserved. add_compile_definitions(RARDLL) -add_compile_definitions(WARN_DLOPEN_FAIL) add_compile_definitions(_FILE_OFFSET_BITS=64) if(WIN32) From 68b4ab44fe5eb35a6e0c2d97293cd62a92ecd24a Mon Sep 17 00:00:00 2001 From: Micah Snyder Date: Fri, 3 May 2024 10:49:40 -0400 Subject: [PATCH 2/2] GitHub Actions: Fix macOS build issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upgrade macOS OpenSSL dependency to use 3 instead of 1.1. Python's pip from Homebrew now refuses to isntall globally: error: externally-managed-environment × This environment is externally managed ╰─> To install Python packages system-wide, try brew install xyz, where xyz is the package you are trying to install. If you wish to install a Python library that isn't in Homebrew, use a virtual environment: python3 -m venv path/to/venv source path/to/venv/bin/activate python3 -m pip install xyz If you wish to install a Python application that isn't in Homebrew, it may be easiest to use 'pipx install xyz', which will manage a virtual environment for you. You can install pipx with brew install pipx You may restore the old behavior of pip by passing the '--break-system-packages' flag to pip, or by adding 'break-system-packages = true' to your pip.conf file. The latter will permanently disable this error. If you disable this error, we STRONGLY recommend that you additionally pass the '--user' flag to pip, or set 'user = true' in your pip.conf file. Failure to do this can result in a broken Homebrew installation. Read more about this behavior here: Using Pipx instead. Making the same change for Ubuntu just in case. --- .github/workflows/cmake.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index 320885e0bf..7b2711fbb5 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -94,13 +94,13 @@ jobs: - uses: actions/checkout@v1 - name: Install Build Tools - run: brew install bison flex + run: brew install bison flex pipx - name: Install Dependencies - run: brew install bzip2 check curl-openssl json-c libxml2 ncurses openssl@1.1 pcre2 zlib + run: brew install bzip2 check curl json-c libxml2 ncurses openssl@3 pcre2 zlib - name: Install pytest for easier to read test results - run: python3 -m pip install pytest + run: pipx install pytest - uses: lukka/get-cmake@v3.21.2 @@ -119,9 +119,9 @@ jobs: # The CMake binaries on the Github Actions machines are (as of this writing) 3.12 run: cmake ${{runner.workspace}}/clamav -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} - -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl@1.1/ - -DOPENSSL_CRYPTO_LIBRARY=/usr/local/opt/openssl@1.1/lib/libcrypto.1.1.dylib - -DOPENSSL_SSL_LIBRARY=/usr/local/opt/openssl@1.1/lib/libssl.1.1.dylib + -DOPENSSL_ROOT_DIR=/opt/homebrew/include/ + -DOPENSSL_CRYPTO_LIBRARY=/opt/homebrew/lib/libcrypto.3.dylib + -DOPENSSL_SSL_LIBRARY=/opt/homebrew/lib/libssl.3.dylib -DENABLE_STATIC_LIB=ON -DENABLE_EXAMPLES=ON @@ -148,13 +148,13 @@ jobs: run: sudo apt-get update - name: Install Build Tools - run: sudo apt-get install -y bison flex valgrind + run: sudo apt-get install -y bison flex valgrind pipx - name: Install Dependencies run: sudo apt-get install -y check libbz2-dev libcurl4-openssl-dev libjson-c-dev libmilter-dev libncurses5-dev libpcre3-dev libssl-dev libxml2-dev zlib1g-dev - name: Install pytest for easier to read test results - run: python3 -m pip install pytest + run: pipx install pytest - uses: lukka/get-cmake@v3.21.2