Skip to content
Permalink
Browse files Browse the repository at this point in the history
make the dump directories owned by root by default
It was discovered that the abrt event scripts create a user-readable
copy of a sosreport file in abrt problem directories, and include
excerpts of /var/log/messages selected by the user-controlled process
name, leading to an information disclosure.

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

Related: #1212868

Signed-off-by: Jakub Filak <jfilak@redhat.com>
  • Loading branch information
Jakub Filak committed May 4, 2015
1 parent 2f0a18b commit 8939398
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 6 deletions.
34 changes: 32 additions & 2 deletions src/daemon/abrt-server.c
Expand Up @@ -15,6 +15,7 @@
with this program; if not, write to the Free Software Foundation, Inc.,
51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
#include "problem_api.h"
#include "libabrt.h"

/* Maximal length of backtrace. */
Expand Down Expand Up @@ -153,7 +154,36 @@ static int run_post_create(const char *dirname)
error_msg("Bad problem directory name '%s', should start with: '%s'", dirname, g_settings_dump_location);
return 400; /* Bad Request */
}
if (!dump_dir_accessible_by_uid(dirname, client_uid))
if (g_settings_privatereports)
{
struct stat statbuf;
if (lstat(dirname, &statbuf) != 0 || !S_ISDIR(statbuf.st_mode))
{
error_msg("Path '%s' isn't directory", dirname);
return 404; /* Not Found */
}
/* Get ABRT's group gid */
struct group *gr = getgrnam("abrt");
if (!gr)
{
error_msg("Group 'abrt' does not exist");
return 500;
}
if (statbuf.st_uid != 0 || !(statbuf.st_gid == 0 || statbuf.st_gid == gr->gr_gid) || statbuf.st_mode & 07)
{
error_msg("Problem directory '%s' isn't owned by root:abrt or others are not restricted from access", dirname);
return 403;
}
struct dump_dir *dd = dd_opendir(dirname, DD_OPEN_READONLY);
const bool complete = dd && problem_dump_dir_is_complete(dd);
dd_close(dd);
if (complete)
{
error_msg("Problem directory '%s' has already been processed", dirname);
return 403;
}
}
else if (!dump_dir_accessible_by_uid(dirname, client_uid))
{
if (errno == ENOTDIR)
{
Expand Down Expand Up @@ -377,7 +407,7 @@ static int create_problem_dir(GHashTable *problem_info, unsigned pid)
/* No need to check the path length, as all variables used are limited,
* and dd_create() fails if the path is too long.
*/
struct dump_dir *dd = dd_create(path, client_uid, DEFAULT_DUMP_DIR_MODE);
struct dump_dir *dd = dd_create(path, g_settings_privatereports ? 0 : client_uid, DEFAULT_DUMP_DIR_MODE);
if (!dd)
{
error_msg_and_die("Error creating problem directory '%s'", path);
Expand Down
5 changes: 5 additions & 0 deletions src/daemon/abrt.conf
Expand Up @@ -43,3 +43,8 @@ AutoreportingEnabled = no
# session; otherwise No.
#
# ShortenedReporting = yes

# Disable this if you want to regular users to own the problem data colleted by
# abrt.
#
PrivateReports = yes
10 changes: 7 additions & 3 deletions src/hooks/abrt-hook-ccpp.c
Expand Up @@ -682,6 +682,9 @@ int main(int argc, char** argv)
}
}

/* If PrivateReports is on, root owns all problem directories */
const uid_t dduid = g_settings_privatereports ? 0 : fsuid;

/* Open a fd to compat coredump, if requested and is possible */
if (setting_MakeCompatCore && ulimit_c != 0)
/* note: checks "user_pwd == NULL" inside; updates core_basename */
Expand Down Expand Up @@ -773,18 +776,19 @@ int main(int argc, char** argv)
goto create_user_core;
}

/* use fsuid instead of uid, so we don't expose any sensitive
* information of suided app in /var/tmp/abrt
/* use dduid (either fsuid or 0) instead of uid, so we don't expose any
* sensitive information of suided app in /var/tmp/abrt
*
* dd_create_skeleton() creates a new directory and leaves ownership to
* the current user, hence, we have to call dd_reset_ownership() after the
* directory is populated.
*/
dd = dd_create_skeleton(path, fsuid, DEFAULT_DUMP_DIR_MODE, /*no flags*/0);
dd = dd_create_skeleton(path, dduid, DEFAULT_DUMP_DIR_MODE, /*no flags*/0);
if (dd)
{
char *rootdir = get_rootdir(pid);

/* This function uses fsuid only for its value. The function stores fsuid in a file name 'uid'*/
dd_create_basic_files(dd, fsuid, NULL);

char source_filename[sizeof("/proc/%lu/somewhat_long_name") + sizeof(long)*3];
Expand Down
2 changes: 2 additions & 0 deletions src/include/libabrt.h
Expand Up @@ -62,6 +62,8 @@ extern bool g_settings_autoreporting;
extern char * g_settings_autoreporting_event;
#define g_settings_shortenedreporting abrt_g_settings_shortenedreporting
extern bool g_settings_shortenedreporting;
#define g_settings_privatereports abrt_g_settings_privatereports
extern bool g_settings_privatereports;


#define load_abrt_conf abrt_load_abrt_conf
Expand Down
8 changes: 8 additions & 0 deletions src/lib/abrt_conf.c
Expand Up @@ -27,6 +27,7 @@ bool g_settings_delete_uploaded = 0;
bool g_settings_autoreporting = 0;
char * g_settings_autoreporting_event = NULL;
bool g_settings_shortenedreporting = 0;
bool g_settings_privatereports = true;

void free_abrt_conf_data()
{
Expand Down Expand Up @@ -102,6 +103,13 @@ static void ParseCommon(map_string_t *settings, const char *conf_filename)
else
g_settings_shortenedreporting = 0;

value = get_map_string_item_or_NULL(settings, "PrivateReports");
if (value)
{
g_settings_privatereports = string_to_bool(value);
remove_map_string_item(settings, "PrivateReports");
}

GHashTableIter iter;
const char *name;
/*char *value; - already declared */
Expand Down
7 changes: 6 additions & 1 deletion src/lib/hooklib.c
Expand Up @@ -410,7 +410,12 @@ char* problem_data_save(problem_data_t *pd)
{
load_abrt_conf();

struct dump_dir *dd = create_dump_dir_from_problem_data(pd, g_settings_dump_location);
struct dump_dir *dd = NULL;

if (g_settings_privatereports)
dd = create_dump_dir_from_problem_data_ext(pd, g_settings_dump_location, 0);
else
dd = create_dump_dir_from_problem_data(pd, g_settings_dump_location);

char *problem_id = NULL;
if (dd)
Expand Down
8 changes: 8 additions & 0 deletions src/plugins/abrt-dump-oops.c
Expand Up @@ -189,6 +189,14 @@ static unsigned create_oops_dump_dirs(GList *oops_list, unsigned oops_cnt)
mode = DEFAULT_DUMP_DIR_MODE;
my_euid = geteuid();
}
if (g_settings_privatereports)
{
if (world_readable_dump)
log("Not going to make dump directories world readable because PrivateReports is on");

mode = DEFAULT_DUMP_DIR_MODE;
my_euid = 0;
}

pid_t my_pid = getpid();
unsigned idx = 0;
Expand Down
8 changes: 8 additions & 0 deletions src/plugins/abrt-dump-xorg.c
Expand Up @@ -82,6 +82,14 @@ static void save_bt_to_dump_dir(const char *bt, const char *exe, const char *rea
mode = DEFAULT_DUMP_DIR_MODE;
my_euid = geteuid();
}
if (g_settings_privatereports)
{
if ((g_opts & OPT_x))
log("Not going to make dump directories world readable because PrivateReports is on");

mode = DEFAULT_DUMP_DIR_MODE;
my_euid = 0;
}

pid_t my_pid = getpid();

Expand Down

0 comments on commit 8939398

Please sign in to comment.