Skip to content

Commit

Permalink
dd: harden functions against directory traversal issues
Browse files Browse the repository at this point in the history
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 <jfilak@redhat.com>
  • Loading branch information
Jakub Filak committed Apr 28, 2015
1 parent 54ecf8d commit 239c4f7
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 7 deletions.
19 changes: 12 additions & 7 deletions src/lib/create_dump_dir.c
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand All @@ -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);
}

Expand Down
22 changes: 22 additions & 0 deletions src/lib/dump_dir.c
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -1075,13 +1088,19 @@ 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);
}

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;
Expand All @@ -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);

Expand Down

0 comments on commit 239c4f7

Please sign in to comment.