Skip to content
Permalink
Browse files Browse the repository at this point in the history
dbus: process only valid sub-directories of the dump location
Must have correct rights and must be a direct sub-directory of the dump
location.

This issue was discovered by Florian Weimer of Red Hat Product Security.

Related: #1214451

Signed-off-by: Jakub Filak <jfilak@redhat.com>
  • Loading branch information
Jakub Filak committed May 4, 2015
1 parent b7f8bd2 commit 6e811d7
Showing 1 changed file with 26 additions and 10 deletions.
36 changes: 26 additions & 10 deletions src/dbus/abrt-dbus.c
Expand Up @@ -132,18 +132,34 @@ static uid_t get_caller_uid(GDBusConnection *connection, GDBusMethodInvocation *
return caller_uid;
}

static bool allowed_problem_dir(const char *dir_name)
bool allowed_problem_dir(const char *dir_name)
{
//HACK HACK HACK! Disabled for now until we fix clients (abrt-gui) to not pass /home/user/.cache/abrt/spool
if (!dir_is_in_dump_location(dir_name))
{
error_msg("Bad problem directory name '%s', should start with: '%s'", dir_name, g_settings_dump_location);
return false;
}

/* We cannot test correct permissions yet because we still need to chown
* dump directories before reporting and Chowing changes the file owner to
* the reporter, which causes this test to fail and prevents users from
* getting problem data after reporting it.
*
* Fortunately, libreport has been hardened against hard link and symbolic
* link attacks and refuses to work with such files, so this test isn't
* really necessary, however, we will use it once we get rid of the
* chowning files.
*
* abrt-server refuses to run post-create on directories that have
* incorrect owner (not "root:(abrt|root)"), incorrect permissions (other
* bits are not 0) and are complete (post-create finished). So, there is no
* way to run security sensitive event scripts (post-create) on crafted
* problem directories.
*/
#if 0
unsigned len = strlen(g_settings_dump_location);

/* If doesn't start with "g_settings_dump_location[/]"... */
if (strncmp(dir_name, g_settings_dump_location, len) != 0
|| (dir_name[len] != '/' && dir_name[len] != '\0')
/* or contains "/." anywhere (-> might contain ".." component) */
|| strstr(dir_name + len, "/.")
) {
if (!dir_has_correct_permissions(dir_name))
{
error_msg("Problem directory '%s' isn't owned by root:abrt or others are not restricted from access", dir_name);
return false;
}
#endif
Expand Down

0 comments on commit 6e811d7

Please sign in to comment.