From 239c4f7d1f47265526b39ad70106767d00805277 Mon Sep 17 00:00:00 2001 From: Jakub Filak Date: Thu, 23 Apr 2015 13:30:15 +0200 Subject: [LIBREPORT PATCH] dd: harden functions against directory traversal issues Test correctness of all accessed dump dir files in all dd* functions. Before this commit, the callers were allowed to pass strings like "../../etc/shadow" in the filename argument of all dd* functions. Related: #1214457 Signed-off-by: Jakub Filak --- src/lib/create_dump_dir.c | 19 ++++++++++++------- src/lib/dump_dir.c | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/lib/create_dump_dir.c b/src/lib/create_dump_dir.c index 4f67523..989a50c 100644 --- a/src/lib/create_dump_dir.c +++ b/src/lib/create_dump_dir.c @@ -42,6 +42,12 @@ struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, return NULL; } + if (!str_is_correct_filename(type)) + { + error_msg(_("'%s' is not correct file name"), FILENAME_ANALYZER); + return NULL; + } + uid_t uid = (uid_t)-1L; char *uid_str = problem_data_get_content_or_NULL(problem_data, FILENAME_UID); @@ -105,6 +111,12 @@ struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, g_hash_table_iter_init(&iter, problem_data); while (g_hash_table_iter_next(&iter, (void**)&name, (void**)&value)) { + if (!str_is_correct_filename(name)) + { + error_msg("Problem data field name contains disallowed chars: '%s'", name); + continue; + } + if (value->flags & CD_FLAG_BIN) { char *dest = concat_path_file(dd->dd_dirname, name); @@ -119,13 +131,6 @@ struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data, continue; } - /* only files should contain '/' and those are handled earlier */ - if (name[0] == '.' || strchr(name, '/')) - { - error_msg("Problem data field name contains disallowed chars: '%s'", name); - continue; - } - dd_save_text(dd, name, value->content); } diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c index 2a65100..0048faf 100644 --- a/src/lib/dump_dir.c +++ b/src/lib/dump_dir.c @@ -345,6 +345,9 @@ static inline struct dump_dir *dd_init(void) int dd_exist(const struct dump_dir *dd, const char *path) { + if (!str_is_correct_filename(path)) + error_msg_and_die("Cannot test existence. '%s' is not a valid file name", path); + char *full_path = concat_path_file(dd->dd_dirname, path); int ret = exist_file_dir(full_path); free(full_path); @@ -1044,6 +1047,13 @@ char* dd_load_text_ext(const struct dump_dir *dd, const char *name, unsigned fla // if (!dd->locked) // error_msg_and_die("dump_dir is not opened"); /* bug */ + if (!str_is_correct_filename(name)) + { + error_msg("Cannot load text. '%s' is not a valid file name", name); + if (!(flags & DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE)) + xfunc_die(); + } + /* Compat with old abrt dumps. Remove in abrt-2.1 */ if (strcmp(name, "release") == 0) name = FILENAME_OS_RELEASE; @@ -1065,6 +1075,9 @@ void dd_save_text(struct dump_dir *dd, const char *name, const char *data) if (!dd->locked) error_msg_and_die("dump_dir is not opened"); /* bug */ + if (!str_is_correct_filename(name)) + error_msg_and_die("Cannot save text. '%s' is not a valid file name", name); + char *full_path = concat_path_file(dd->dd_dirname, name); save_binary_file(full_path, data, strlen(data), dd->dd_uid, dd->dd_gid, dd->mode); free(full_path); @@ -1075,6 +1088,9 @@ void dd_save_binary(struct dump_dir* dd, const char* name, const char* data, uns if (!dd->locked) error_msg_and_die("dump_dir is not opened"); /* bug */ + if (!str_is_correct_filename(name)) + error_msg_and_die("Cannot save binary. '%s' is not a valid file name", name); + char *full_path = concat_path_file(dd->dd_dirname, name); save_binary_file(full_path, data, size, dd->dd_uid, dd->dd_gid, dd->mode); free(full_path); @@ -1082,6 +1098,9 @@ void dd_save_binary(struct dump_dir* dd, const char* name, const char* data, uns long dd_get_item_size(struct dump_dir *dd, const char *name) { + if (!str_is_correct_filename(name)) + error_msg_and_die("Cannot get item size. '%s' is not a valid file name", name); + long size = -1; char *iname = concat_path_file(dd->dd_dirname, name); struct stat statbuf; @@ -1106,6 +1125,9 @@ int dd_delete_item(struct dump_dir *dd, const char *name) if (!dd->locked) error_msg_and_die("dump_dir is not opened"); /* bug */ + if (!str_is_correct_filename(name)) + error_msg_and_die("Cannot delete item. '%s' is not a valid file name", name); + char *path = concat_path_file(dd->dd_dirname, name); int res = unlink(path); -- 1.8.3.1