From 2909a8253c2471c4a01e495dc713c0579a0a0a40 Mon Sep 17 00:00:00 2001 From: Daniel Markstedt Date: Mon, 4 May 2026 21:30:10 +0200 Subject: [PATCH] CVE-2026-44051: afpd: validate symlink targets from FinderInfo Three secure helper functions are introduced for validating symlinks: has_dotdot_component() to reject any ".." component in the target path, path_is_inside_volume() to verify that a resolved path falls within the volume root, and symlink_target_safe() which combines both checks: it rejects absolute targets and targets with ".." components outright, then resolves the remaining relative path with realpath_safe() and confirms the result is inside the volume before permitting the symlink creation. Rewrite test426 in the spectest suite to validate the rejection of dangling symlinks, rather than the opposite. Reported-by: @00redbeer Signed-off-by: Daniel Markstedt --- etc/afpd/file.c | 82 ++++++++++++++++++ test/testsuite/T2_FPSetFileParms.c | 133 ++++++++++++----------------- 2 files changed, 136 insertions(+), 79 deletions(-) diff --git a/etc/afpd/file.c b/etc/afpd/file.c index cfabaa78b..ff9a90a0b 100644 --- a/etc/afpd/file.c +++ b/etc/afpd/file.c @@ -73,6 +73,83 @@ static const uint8_t old_ufinderi[] = { 'T', 'E', 'X', 'T', 'U', 'N', 'I', 'X' }; +static int has_dotdot_component(const char *path) +{ + const char *p = path; + + while (*p) { + size_t len; + const char *slash = strchr(p, '/'); + len = slash ? (size_t)(slash - p) : strlen(p); + + if (len == 2 && p[0] == '.' && p[1] == '.') { + return 1; + } + + if (!slash) { + break; + } + + p = slash + 1; + } + + return 0; +} + +static int path_is_inside_volume(const struct vol *vol, const char *path) +{ + size_t volpath_len = strlen(vol->v_path); + + if (strncmp(path, vol->v_path, volpath_len) != 0) { + return 0; + } + + return path[volpath_len] == '\0' || path[volpath_len] == '/'; +} + +static int symlink_target_safe(const struct vol *vol, + const char *link_path, + const char *target) +{ + char target_path[MAXPATHLEN + 1]; + char link_dir[MAXPATHLEN + 1]; + char *slash; + char *resolved_target = NULL; + int safe = 0; + + if (target[0] == '/' || has_dotdot_component(target)) { + return 0; + } + + if (strlcpy(link_dir, link_path, sizeof(link_dir)) >= sizeof(link_dir)) { + return 0; + } + + slash = strrchr(link_dir, '/'); + + if (slash == link_dir) { + slash[1] = '\0'; + } else if (slash) { + *slash = '\0'; + } else { + strlcpy(link_dir, ".", sizeof(link_dir)); + } + + if (snprintf(target_path, sizeof(target_path), "%s/%s", link_dir, target) + >= sizeof(target_path)) { + return 0; + } + + resolved_target = realpath_safe(target_path); + + if (resolved_target) { + safe = path_is_inside_volume(vol, resolved_target); + free(resolved_target); + } + + return safe; +} + /* ---------------------- */ static int default_type(void *finder) @@ -1049,6 +1126,11 @@ int setfilparams(const AFPObj *obj, struct vol *vol, symbuf[len] = 0; + if (!symlink_target_safe(vol, path->u_name, symbuf)) { + err = AFPERR_ACCESS; + goto setfilparam_done; + } + if (symlink(symbuf, path->u_name) != 0) { err = AFPERR_MISC; goto setfilparam_done; diff --git a/test/testsuite/T2_FPSetFileParms.c b/test/testsuite/T2_FPSetFileParms.c index 6002cb6f3..12b9fb796 100644 --- a/test/testsuite/T2_FPSetFileParms.c +++ b/test/testsuite/T2_FPSetFileParms.c @@ -5,57 +5,6 @@ #include "afphelper.h" #include "testhelper.h" -/* ------------------------ */ -static int afp_symlink(char *oldpath, char *newpath) -{ - int ofs = 3 * sizeof(uint16_t); - struct afp_filedir_parms filedir; - uint16_t bitmap; - uint16_t vol = VolID; - const DSI *dsi; - int fork = 0; - dsi = &Conn->dsi; - - if (FPCreateFile(Conn, vol, 0, DIRDID_ROOT, newpath)) { - return -1; - } - - fork = FPOpenFork(Conn, vol, OPENFORK_DATA, 0, DIRDID_ROOT, newpath, - OPENACC_WR | OPENACC_RD); - - if (!fork) { - return -1; - } - - if (FPWrite(Conn, fork, 0, (int)strlen(oldpath), oldpath, 0)) { - return -1; - } - - if (FPCloseFork(Conn, fork)) { - return -1; - } - - fork = 0; - bitmap = (1 << DIRPBIT_ATTR) | (1 << FILPBIT_FINFO) | - (1 << DIRPBIT_CDATE) | (1 << DIRPBIT_MDATE) | - (1 << DIRPBIT_LNAME) | (1 << DIRPBIT_PDID) | (1 << FILPBIT_FNUM); - - if (FPGetFileDirParams(Conn, vol, DIRDID_ROOT, newpath, bitmap, 0)) { - return -1; - } - - filedir.isdir = 0; - afp_filedir_unpack(&filedir, dsi->data + ofs, bitmap, 0); - memcpy(filedir.finder_info, "slnkrhap", 8); - bitmap = (1 << FILPBIT_FINFO); - - if (FPSetFileParams(Conn, vol, DIRDID_ROOT, newpath, bitmap, &filedir)) { - return -1; - } - - return 0; -} - /* ------------------------- */ STATIC void test89() { @@ -149,12 +98,15 @@ STATIC void test120() STATIC void test426() { char *name = "t426 Symlink"; + char *target = "t426 dest"; int ofs = 3 * sizeof(uint16_t); struct afp_filedir_parms filedir; uint16_t bitmap; uint16_t vol = VolID; - DSI *dsi; + const DSI *dsi; int fork = 0; + int created = 0; + int len; unsigned int ret; char temp[MAXPATHLEN]; struct stat st; @@ -172,57 +124,80 @@ STATIC void test426() goto test_exit; } - if (afp_symlink("t426 dest", name)) { + if (FPCreateFile(Conn, vol, 0, DIRDID_ROOT, name)) { test_nottested(); goto test_exit; } - /* Check if volume uses option 'followsymlinks' */ - sprintf(temp, "%s/%s", Path, name); + created = 1; + fork = FPOpenFork(Conn, vol, OPENFORK_DATA, 0, DIRDID_ROOT, name, + OPENACC_WR | OPENACC_RD); - if (lstat(temp, &st)) { - if (!Quiet) { - fprintf(stdout, "\tFAILED stat( %s ) %s\n", temp, strerror(errno)); - } + if (!fork) { + test_nottested(); + goto test_exit; + } - test_failed(); + if (FPWrite(Conn, fork, 0, (int)strlen(target), target, 0)) { + test_nottested(); + goto test_exit; } - if (!S_ISLNK(st.st_mode)) { - test_skipped(T_NOSYML); + if (FPCloseFork(Conn, fork)) { + test_nottested(); goto test_exit; } - fork = FPOpenFork(Conn, vol, OPENFORK_DATA, 0, DIRDID_ROOT, name, - OPENACC_WR | OPENACC_RD); + fork = 0; + bitmap = (1 << DIRPBIT_ATTR) | (1 << FILPBIT_FINFO) | + (1 << DIRPBIT_CDATE) | (1 << DIRPBIT_MDATE) | + (1 << DIRPBIT_LNAME) | (1 << DIRPBIT_PDID) | (1 << FILPBIT_FNUM); - if (!fork) { - test_failed(); - } else { - char *ln2 = "t426 dest 2"; - ret = FPWrite_ext(Conn, fork, 0, (off_t)strlen(ln2), ln2, 0); + if (FPGetFileDirParams(Conn, vol, DIRDID_ROOT, name, bitmap, 0)) { + test_nottested(); + goto test_exit; + } - if (not_valid_bitmap(ret, BITERR_ACCESS | BITERR_MISC, AFPERR_MISC)) { - test_failed(); - } + filedir.isdir = 0; + afp_filedir_unpack(&filedir, dsi->data + ofs, bitmap, 0); + memcpy(filedir.finder_info, "slnkrhap", 8); + bitmap = (1 << FILPBIT_FINFO); + ret = FPSetFileParams(Conn, vol, DIRDID_ROOT, name, bitmap, &filedir); - FPCloseFork(Conn, fork); + if (ret != htonl(AFPERR_ACCESS)) { + test_failed(); } - fork = FPOpenFork(Conn, vol, OPENFORK_DATA, 0, DIRDID_ROOT, name, OPENACC_RD); + len = snprintf(temp, sizeof(temp), "%s/%s", Path, name); - if (!fork) { - /* Trying to open the linked file? */ + if (len < 0 || len >= (int)sizeof(temp)) { + test_failed(); + goto test_exit; + } + + if (lstat(temp, &st) == 0) { + if (S_ISLNK(st.st_mode)) { + test_failed(); + } + } else if (errno != ENOENT) { test_failed(); } +test_exit: + if (fork) { FPCloseFork(Conn, fork); } - FAIL(FPDelete(Conn, vol, DIRDID_ROOT, name)) -test_exit: - exit_test("FPSetFileParms:test426: Create a dangling symlink"); + if (created) { + ret = FPDelete(Conn, vol, DIRDID_ROOT, name); + + if (ret && ret != htonl(AFPERR_NOOBJ)) { + test_failed(); + } + } + + exit_test("FPSetFileParms:test426: Reject a dangling symlink"); } /* ----------- */