From: Timo Sirainen Date: Mon, 4 May 2026 13:09:37 +0000 (+0000) Subject: [PATCH 1/2] lib-sieve: storage: file - Refuse symlinks escaping personal storage... X-Git-Tag: archive/raspbian/1%2.4.1+dfsg1-6+rpi1+deb13u7^2~95 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=858e97c48245c42fedb7cd6cc8b0b38934be37fb;p=dovecot.git [PATCH 1/2] lib-sieve: storage: file - Refuse symlinks escaping personal storage directory Pigeonhole's file storage followed any symlink encountered while resolving a script path, including symlinks in personal (user-writable) storage whose target lay outside the storage directory. In some non-recommended configurations a user could exploit this through the include extension: an "include :personal name;" lookup of ~/sieve/name.sieve transparently followed a user-placed to e.g. another user's file readable by the mail process, leaking its contents (or causing it to be parsed as Sieve). Normally this shouldn't be possible, because sieve processes shouldn't have any more privileges to read files than the local system user creating the symlink. Open the canonical (realpath'd) personal storage directory at storage init time and keep an O_DIRECTORY|O_CLOEXEC fd to it. Resolve script content reads through this fd by routing sieve_file_script_get_stream() via a new sieve_file_storage_open_safe() wrapper around t_openat_safe(), which: - opens each path component with O_NOFOLLOW so symlinks are detected explicitly rather than transparently followed; - follows symlinks only when their (recursively resolved) target stays beneath dir_fd, refusing absolute targets and `..` past the storage root with ELOOP; - caps the symlink-hop count to bound resolution time. Anchoring at dir_fd makes the lookup TOCTOU-safe even when intermediate path components are mutated on disk concurrently: resolution stays relative to the original directory inode and the safe walker still rejects any target that leaves it. Apply the protection only when storage->is_personal is set; admin-managed global storage is trusted and may legitimately use cross-boundary symlinks. Single-file storages (is_file=TRUE) keep dir_fd at -1 and fall back to the existing open path. Also guard the dir_fd open with S_ISDIR() to handle the autodetect quirk where storage_path can refer to a regular file even when is_file is FALSE. Gbp-Pq: Name 0001-lib-sieve-storage-file-Refuse-symlinks-escaping-pers.patch --- diff --git a/pigeonhole/src/lib-sieve/storage/file/sieve-file-script.c b/pigeonhole/src/lib-sieve/storage/file/sieve-file-script.c index 05f8ea3..52e752a 100644 --- a/pigeonhole/src/lib-sieve/storage/file/sieve-file-script.c +++ b/pigeonhole/src/lib-sieve/storage/file/sieve-file-script.c @@ -441,15 +441,44 @@ sieve_file_script_get_stream(struct sieve_script *script, { struct sieve_file_script *fscript = container_of(script, struct sieve_file_script, script); + struct sieve_file_storage *fstorage = + container_of(script->storage, struct sieve_file_storage, + storage); struct stat st; struct istream *result; + const char *error; int fd; - fd = open(fscript->path, O_RDONLY); - if (fd < 0) { - sieve_file_script_handle_error(fscript, "open", fscript->path, - fscript->script.name); - return -1; + /* For directory-based storage, open the script via the storage + directory fd so that path resolution refuses to follow symlinks + whose (recursive) target leaves the storage directory. + Single-file storages have no dir_fd, so fall back to plain open(). */ + if (fstorage->dir_fd >= 0 && fscript->filename != NULL && + *fscript->filename != '\0') { + if (sieve_file_storage_open_safe(fstorage, fscript->filename, + O_RDONLY, &fd, &error) < 0) { + if (errno == ELOOP) { + sieve_script_set_critical( + script, + "Failed to open sieve script: %s", + error); + script->storage->error_code = + SIEVE_ERROR_NO_PERMISSION; + return -1; + } + sieve_file_script_handle_error(fscript, "open", + fscript->path, + fscript->script.name); + return -1; + } + } else { + fd = open(fscript->path, O_RDONLY); + if (fd < 0) { + sieve_file_script_handle_error(fscript, "open", + fscript->path, + fscript->script.name); + return -1; + } } if (fstat(fd, &st) != 0) { diff --git a/pigeonhole/src/lib-sieve/storage/file/sieve-file-storage.c b/pigeonhole/src/lib-sieve/storage/file/sieve-file-storage.c index 75d62cb..e65ae5e 100644 --- a/pigeonhole/src/lib-sieve/storage/file/sieve-file-storage.c +++ b/pigeonhole/src/lib-sieve/storage/file/sieve-file-storage.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -42,6 +43,33 @@ sieve_file_storage_path_extend(struct sieve_file_storage *fstorage, return t_strconcat(path, "/", filename , NULL); } +int sieve_file_storage_open_safe(struct sieve_file_storage *fstorage, + const char *path, int flags, int *fd_r, + const char **error_r) +{ + const char *walk_error; + int fd; + + *fd_r = -1; + + if (fstorage->dir_fd < 0) { + *error_r = "storage directory fd not available"; + errno = ENOTSUP; + return -1; + } + + fd = t_openat_safe(fstorage->dir_fd, path, flags, &walk_error); + if (fd < 0) { + *error_r = t_strdup_printf( + "Failed to open '%s/%s': %s", + fstorage->path, path, walk_error); + return -1; + } + + *fd_r = fd; + return 0; +} + /* * */ @@ -215,10 +243,19 @@ static struct sieve_storage *sieve_file_storage_alloc(void) fstorage = p_new(pool, struct sieve_file_storage, 1); fstorage->storage = sieve_file_storage; fstorage->storage.pool = pool; + fstorage->dir_fd = -1; return &fstorage->storage; } +static void sieve_file_storage_destroy(struct sieve_storage *storage) +{ + struct sieve_file_storage *fstorage = + container_of(storage, struct sieve_file_storage, storage); + + i_close_fd(&fstorage->dir_fd); +} + static int sieve_file_storage_get_full_path(struct sieve_file_storage *fstorage, const char **storage_path) @@ -436,6 +473,42 @@ sieve_file_storage_init_common(struct sieve_file_storage *fstorage, fstorage->link_path = p_strdup(storage->pool, link_path); } + + /* For personal (user-writable) storage, open a fd to the + canonical storage directory. This fd anchors TOCTOU-safe + path resolution for script lookups: the inode it refers to + cannot change underneath us, so even if the user mutates + intermediate path components on disk afterwards, we still + resolve relative to the original directory. The walker that + uses this fd refuses script paths that escape the storage + directory through symlinks or `..`, which would otherwise + let an unprivileged user redirect e.g. include "name" to a + file outside their own sieve storage. + + Non-personal (e.g. admin-configured global) storage is + trusted; dir_fd stays unset there so legitimate + admin-managed symlinks crossing the storage boundary keep + working. + + Only open the fd when storage_path actually resolves to a + directory: autodetect can leave is_file=FALSE while + storage_path still points at a regular file (see the + storage_path == active_path fallback in + sieve_file_storage_do_autodetect()). In that case there is + no directory to anchor to and lookups by name will fail + with ENOENT later anyway. */ + if (storage->is_personal && + S_ISDIR(fstorage->st.st_mode)) { + fstorage->dir_fd = open(storage_path, + O_RDONLY | O_DIRECTORY | + O_CLOEXEC); + if (fstorage->dir_fd < 0) { + sieve_storage_set_critical(storage, + "Failed to open storage directory: " + "open(%s) failed: %m", storage_path); + return -1; + } + } } fstorage->path = p_strdup(storage->pool, storage_path); @@ -904,6 +977,7 @@ const struct sieve_storage sieve_file_storage = { .v = { .alloc = sieve_file_storage_alloc, .init = sieve_file_storage_init, + .destroy = sieve_file_storage_destroy, .autodetect = sieve_file_storage_autodetect, diff --git a/pigeonhole/src/lib-sieve/storage/file/sieve-file-storage.h b/pigeonhole/src/lib-sieve/storage/file/sieve-file-storage.h index 0187beb..a4097f8 100644 --- a/pigeonhole/src/lib-sieve/storage/file/sieve-file-storage.h +++ b/pigeonhole/src/lib-sieve/storage/file/sieve-file-storage.h @@ -41,6 +41,13 @@ struct sieve_file_storage { time_t prev_mtime; + /* fd referencing the canonical (realpath'd) storage directory. Used to + anchor TOCTOU-safe path resolution for script files: lookups that + escape this directory via symlinks (or `..`) are refused. -1 if the + storage is a single-file storage or this protection is unavailable. + */ + int dir_fd; + bool is_file:1; }; @@ -60,6 +67,31 @@ int sieve_file_storage_init_from_path(struct sieve_instance *svinst, int sieve_file_storage_pre_modify(struct sieve_storage *storage); +/* Open a script file that lives under the storage directory, refusing any + path resolution that escapes that directory through symlinks or `..`. + + The path is resolved component-by-component using openat() relative to + fstorage->dir_fd, with O_NOFOLLOW per component. Symlinks are followed + only if their (recursively resolved) target also stays beneath dir_fd; + absolute symlink targets and `..` past the storage root are refused. + + Anchoring the resolution at dir_fd makes the check TOCTOU-safe: even if + the user mutates path components on disk between calls, dir_fd still + refers to the original directory inode. + + `flags` is OR-ed into the openat() call for the final component (e.g. + O_RDONLY). O_NOFOLLOW and O_CLOEXEC are added automatically. + + Returns 0 on success and stores the new fd in *fd_r. Returns -1 on + failure with errno set; *error_r is set to a descriptive message. + errno=ELOOP indicates either too many symlinks or an attempted escape. + errno=ENOTSUP indicates fstorage->dir_fd is not available; the caller + may fall back to opening fstorage->path directly. + */ +int sieve_file_storage_open_safe(struct sieve_file_storage *fstorage, + const char *path, int flags, int *fd_r, + const char **error_r); + /* Active script */ int sieve_file_storage_active_replace_link(struct sieve_file_storage *fstorage,