ClamD: Fix clamdscan error with broken symlink on macOS

The `realpath()` function on macOS will fail when scanning a symlink
that doesn't exist because it tries to determine the real path of the
thing the symlink points to and fails when that thing doesn't exist.
This behavior is different than on Linux or FreeBSD Unix.

I'm fixing this by opening the symlink with `O_SYMLINK` then getting
realpath of the link using `cli_get_filepath_from_filedesc()`, much
like we do on Windows.

Side note: I did try using macOS's _DARWIN_BETTER_REALPATH variant to
see if that resolved the issue, but it didn't seem to.

This resolves https://bugzilla.clamav.net/show_bug.cgi?id=12792

This commit also removes the problematic "access denied" clamd test from
the test suite. This commit broke that test on macOS because the error
message when clamdscan fails to `open()` the file to check the
realpath() is different than the error message when clamd tries to scan
it, but has access denied.

(It's like "Permission denied" instead of "Access denied")

It's not worth #ifdef'ing around macOS to get a test pass because this
test is already causing problems in 32-bit Docker environments where
access isn't actually denied to the user (!). Honestly, it's not worth
keeping the test that simply verifies ClamD's error message when denied
matches expectations.

I also switched to use the C_DARWIN macro instead of __APPLE__ because
I kno C_DARWIN is defined in clamav-config.h on macOS, and I found that
__APPLE__ wasn't defined when I tested.

libclamav/others_common.c

@@ -1402,7 +1402,8 @@ cl_error_t cli_get_filepath_from_filedesc(int desc, char **filepath)
         goto done;
     }
 
-#elif __APPLE__
+#elif C_DARWIN
+
     char fname[PATH_MAX];
     memset(&fname, 0, PATH_MAX);
 
@@ -1462,6 +1463,8 @@ cl_error_t cli_realpath(const char *file_name, char **real_filename)
     cl_error_t status    = CL_EARG;
 #ifdef _WIN32
     HANDLE hFile = INVALID_HANDLE_VALUE;
+#elif C_DARWIN
+    int fd = -1;
 #endif
 
     cli_dbgmsg("Checking realpath of %s\n", file_name);
@@ -1471,17 +1474,8 @@ cl_error_t cli_realpath(const char *file_name, char **real_filename)
         goto done;
     }
 
-#ifndef _WIN32
-
-    real_file_path = realpath(file_name, NULL);
-    if (NULL == real_file_path) {
-        status = CL_EMEM;
-        goto done;
-    }
-
-    status = CL_SUCCESS;
+#ifdef _WIN32
 
-#else
     hFile = CreateFileA(file_name,                  // file to open
                         GENERIC_READ,               // open for reading
                         FILE_SHARE_READ,            // share for reading
@@ -1497,6 +1491,39 @@ cl_error_t cli_realpath(const char *file_name, char **real_filename)
 
     status = cli_get_filepath_from_handle(hFile, &real_file_path);
 
+#elif C_DARWIN
+
+    /* Using the filepath from filedesc method on macOS because
+       realpath will fail to check the realpath of a symbolic link if
+       the link doesn't point to anything.
+       Plus, we probably don't wan tot follow the link in this case anyways,
+       so this will check the realpath of the link, and not of the thing the
+       link points to. */
+    fd = open(file_name, O_RDONLY | O_SYMLINK);
+    if (fd == -1) {
+        char err[128];
+        cli_strerror(errno, err, sizeof(err));
+        if (errno == EACCES) {
+            status = CL_EACCES;
+        } else {
+            status = CL_EOPEN;
+        }
+        cli_dbgmsg("Can't open file %s: %s\n", file_name, err);
+        goto done;
+    }
+
+    status = cli_get_filepath_from_filedesc(fd, &real_file_path);
+
+#else
+
+    real_file_path = realpath(file_name, NULL);
+    if (NULL == real_file_path) {
+        status = CL_EMEM;
+        goto done;
+    }
+
+    status = CL_SUCCESS;
+
 #endif
 
     *real_filename = real_file_path;
@@ -1507,6 +1534,10 @@ done:
     if (hFile != INVALID_HANDLE_VALUE) {
         CloseHandle(hFile);
     }
+#elif C_DARWIN
+    if (fd != -1) {
+        close(fd);
+    }
 #endif
 
     return status;

unit_tests/check_clamd.c

@@ -148,11 +148,6 @@ static void conn_teardown(void)
 
 #define NONEXISTENT_REPLY NONEXISTENT ": File path check failure: No such file or directory. ERROR"
 
-#ifndef _WIN32
-#define ACCDENIED OBJDIR PATHSEP "accdenied"
-#define ACCDENIED_REPLY ": Access denied. ERROR"
-#endif
-
 static int isroot = 0;
 
 static void commands_setup(void)
@@ -169,22 +164,7 @@ static void commands_setup(void)
     ck_assert_msg(fd == -1, "Nonexistent file exists!\n");
 
 #ifndef _WIN32
-    /*
-     * Prepare a file path that is write-only.
-     * Note: doesn't work on Windows (O_RWONLY is implicitly readable), so we skip this test on Windows.
-     */
-    fd = open(ACCDENIED, O_CREAT | O_WRONLY, S_IWUSR);
-    ck_assert_msg(fd != -1,
-                  "Failed to create file for access denied tests: %s\n", strerror(errno));
-    ck_assert_msg(fchmod(fd, S_IWUSR) != -1,
-                  "Failed to chmod: %s\n", strerror(errno));
-    /* must not be empty file */
-    ck_assert_msg((size_t)write(fd, nonempty, strlen(nonempty)) == strlen(nonempty),
-                  "Failed to write into testfile: %s\n", strerror(errno));
-    close(fd);
-
-    /* Prepare the "isroot" global so we can skip the access-denied tests when run as root
-       because... you know, root will ignore permissions and still read the file. */
+    /* Prepare the "isroot" global so we can skip some tests when run as root */
     if (!geteuid()) {
         isroot = 1;
     }
@@ -233,12 +213,6 @@ static struct basic_test {
     {"SCAN " NONEXISTENT, NULL, NONEXISTENT_REPLY, 1, 0, IDS_OK},
     {"CONTSCAN " NONEXISTENT, NULL, NONEXISTENT_REPLY, 1, 0, IDS_REJECT},
     {"MULTISCAN " NONEXISTENT, NULL, NONEXISTENT_REPLY, 1, 0, IDS_REJECT},
-/* commands for access denied files */
-#ifndef _WIN32
-    {"SCAN " ACCDENIED, NULL, ACCDENIED_REPLY, 1, 1, IDS_OK},
-    {"CONTSCAN " ACCDENIED, NULL, ACCDENIED_REPLY, 1, 1, IDS_REJECT},
-    {"MULTISCAN " ACCDENIED, NULL, ACCDENIED_REPLY, 1, 1, IDS_REJECT},
-#endif
     /* commands with invalid/missing arguments */
     {"SCAN", NULL, UNKNOWN_REPLY, 1, 0, IDS_REJECT},
     {"CONTSCAN", NULL, UNKNOWN_REPLY, 1, 0, IDS_REJECT},