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 e39d112ad8d69e2d3d248b8943e2a0c3624ebad3..ce4ec4f5a53f55c9a3dc4a567a342c149dfa009f 100644 (file)
--- 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 31af57867e8bd3d493bbf7027f9c733d84170a06..f39c86cdf9878e6f270e33f65cc3e21b9845a96e 100644 (file)
--- 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 (file)
index 0000000..b95a59f
--- /dev/null
+++ b/src/lib-dict-extra/test-dict-fs.c
@@ -0,0 +1,104 @@
+#include <errno.h>
+#include <sys/stat.h>
+#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;
+}