From: Aki Tuomi Date: Tue, 8 Jun 2021 06:13:25 +0000 (+0300) Subject: lib-dict-extra: dict-fs - Escape unsafe paths X-Git-Tag: 2.3.16~27 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9b04e6fbfb5ea3d50ce9b8a26fbae9b68e447995;p=thirdparty%2Fdovecot%2Fcore.git lib-dict-extra: dict-fs - Escape unsafe paths Change any path components that are `.` or `..` to `...` and `....`. Prevents path traversal attacks. --- diff --git a/src/lib-dict-extra/Makefile.am b/src/lib-dict-extra/Makefile.am index e39d112ad8..ce4ec4f5a5 100644 --- a/src/lib-dict-extra/Makefile.am +++ b/src/lib-dict-extra/Makefile.am @@ -4,6 +4,7 @@ dict_drivers = @dict_drivers@ AM_CPPFLAGS = \ -I$(top_srcdir)/src/lib \ + -I$(top_srcdir)/src/lib-test \ -I$(top_srcdir)/src/lib-dict \ -I$(top_srcdir)/src/lib-fs \ -I$(top_srcdir)/src/lib-settings @@ -13,3 +14,22 @@ libdict_extra_la_SOURCES = \ dict-register.c NOPLUGIN_LDFLAGS = + +test_programs = \ + test-dict-fs + +noinst_PROGRAMS = $(test_programs) + +test_libs = \ + ../lib-test/libtest.la \ + ../lib-dict/libdict.la \ + ../lib/liblib.la + +test_dict_fs_SOURCES = test-dict-fs.c +test_dict_fs_LDADD = $(noinst_LTLIBRARIES) ../lib-fs/libfs.la $(test_libs) $(MODULE_LIBS) +test_dict_fs_DEPENDENCIES = $(noinst_LTLIBRARIES) ../lib-fs/libfs.la $(test_libs) + +check-local: + for bin in $(test_programs) $(check_PROGRAMS); do \ + if ! $(RUN_TEST) ./$$bin; then exit 1; fi; \ + done diff --git a/src/lib-dict-extra/dict-fs.c b/src/lib-dict-extra/dict-fs.c index 31af57867e..f39c86cdf9 100644 --- a/src/lib-dict-extra/dict-fs.c +++ b/src/lib-dict-extra/dict-fs.c @@ -68,8 +68,37 @@ static void fs_dict_deinit(struct dict *_dict) i_free(dict); } +/* Remove unsafe paths */ +static const char *fs_dict_escape_key(const char *key) +{ + const char *ptr; + string_t *new_key = NULL; + /* we take the slow path always if we see potential + need for escaping */ + while ((ptr = strstr(key, "/.")) != NULL) { + /* move to the first dot */ + const char *ptr2 = ptr + 1; + /* find position of non-dot */ + while (*ptr2 == '.') ptr2++; + if (new_key == NULL) + new_key = t_str_new(strlen(key)); + str_append_data(new_key, key, ptr - key); + /* if ptr2 is / or end of string, escape */ + if (*ptr2 == '/' || *ptr2 == '\0') + str_append(new_key, "/..."); + else + str_append(new_key, "/."); + key = ptr + 2; + } + if (new_key == NULL) + return key; + str_append(new_key, key); + return str_c(new_key); +} + static const char *fs_dict_get_full_key(struct fs_dict *dict, const char *key) { + key = fs_dict_escape_key(key); if (str_begins(key, DICT_PATH_SHARED)) return key + strlen(DICT_PATH_SHARED); else if (str_begins(key, DICT_PATH_PRIVATE)) { diff --git a/src/lib-dict-extra/test-dict-fs.c b/src/lib-dict-extra/test-dict-fs.c new file mode 100644 index 0000000000..b95a59f794 --- /dev/null +++ b/src/lib-dict-extra/test-dict-fs.c @@ -0,0 +1,104 @@ +#include +#include +#include "lib.h" +#include "unlink-directory.h" +#include "test-common.h" +#include "dict-private.h" + +static void test_dict_set_get(struct dict *dict, const char *key, + const char *value) +{ + const char *got_value, *error; + struct dict_transaction_context *t = dict_transaction_begin(dict); + dict_set(t, key, value); + if (dict_transaction_commit(&t, &error) < 0) + i_fatal("dict_transaction_commit(%s) failed: %s", key, error); + if (dict_lookup(dict, pool_datastack_create(), key, &got_value, + &error) < 0) + i_fatal("dict_lookup(%s) failed: %s", key, error); + test_assert_strcmp(got_value, value); +} + +static bool test_file_exists(const char *path) +{ + struct stat st; + if (stat(path, &st) < 0) { + if (ENOTFOUND(errno)) return FALSE; + i_fatal("stat(%s) failed: %m", path); + } + return TRUE; +} + +static void test_dict_fs_set_get(void) +{ + test_begin("dict-fs get/set"); + const char *error; + struct dict *dict; + struct dict_settings set = { + .username = "testuser", + .base_dir = ".", + }; + if (dict_init("fs:posix:prefix=.test-dict/", &set, &dict, &error) < 0) + i_fatal("dict_init() failed: %s", error); + + /* shared paths */ + struct { + const char *key; + const char *path; + } test_cases[] = { + { "shared/./key", ".test-dict/.../key" }, + { "shared/../key", ".test-dict/..../key" }, + { "shared/.../key", ".test-dict/...../key" }, + { "shared/..../key", ".test-dict/....../key" }, + { "shared/...../key", ".test-dict/......./key" }, + { "shared/key/.", ".test-dict/key/..." }, + { "shared/key/..", ".test-dict/key/...." }, + { "shared/key/...", ".test-dict/key/....." }, + { "shared/key/....", ".test-dict/key/......" }, + { "shared/key/.....", ".test-dict/key/......." }, + { "shared/key/.key", ".test-dict/key/.key" }, + { "shared/key/..key", ".test-dict/key/..key" }, + { "shared/key/...key", ".test-dict/key/...key" }, + { "shared/.key/key", ".test-dict/.key/key" }, + { "shared/..key/key", ".test-dict/..key/key" }, + { "shared/...key/key", ".test-dict/...key/key" }, + }; + for (size_t i = 0; i < N_ELEMENTS(test_cases); i++) { + test_dict_set_get(dict, test_cases[i].key, "1"); + test_assert(test_file_exists(test_cases[i].path)); + } + + /* per user paths */ + test_dict_set_get(dict, "priv/value", "priv1"); + test_assert(test_file_exists(".test-dict/testuser/value")); + test_dict_set_get(dict, "priv/path/with/value", "priv2"); + test_assert(test_file_exists(".test-dict/testuser/path/with/value")); + + /* check that dots work correctly */ + test_dict_set_get(dict, "shared/../test-dict-fs.c", "3"); + test_assert(test_file_exists(".test-dict/..../test-dict-fs.c")); + test_dict_set_get(dict, "shared/./test", "4"); + test_assert(test_file_exists(".test-dict/.../test")); + test_dict_set_get(dict, "shared/.test", "5"); + test_assert(test_file_exists(".test-dict/.test")); + test_dict_set_get(dict, "shared/..test", "6"); + test_assert(test_file_exists(".test-dict/..test")); + dict_deinit(&dict); + + if (unlink_directory(".test-dict", UNLINK_DIRECTORY_FLAG_RMDIR, &error) < 0) + i_fatal("unlink_directory(.test_dict) failed: %s", error); + test_end(); +} + +int main(void) +{ + static void (*const test_functions[])(void) = { + test_dict_fs_set_get, + NULL + }; + int ret; + dict_driver_register(&dict_driver_fs); + ret = test_run(test_functions); + dict_driver_unregister(&dict_driver_fs); + return ret; +}