From a8ee7db7bdd391154918a9fef814d315f3a092b6 Mon Sep 17 00:00:00 2001

From: Jakub Filak <jfilak@redhat.com>

Date: Mon, 20 Apr 2015 15:15:40 +0200

Subject: [PATCH] upload: validate and sanitize uploaded dump directories

It was discovered that, when moving problem reports from

/var/spool/abrt-upload to /var/spool/abrt or /var/tmp/abrt,

abrt-handle-upload does not verify that the new problem directory

has appropriate permissions and does not contain symbolic links.  A

crafted problem report exposes other parts of abrt to attack, and

the abrt-handle-upload script allows to overwrite arbitrary files.

Acknowledgement:

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

Related: #1212953

Signed-off-by: Jakub Filak <jfilak@redhat.com>

---

 src/daemon/abrt-handle-upload.in | 80 +++++++++++++++++++++++++++++++++++-----

 1 file changed, 71 insertions(+), 9 deletions(-)

diff --git a/src/daemon/abrt-handle-upload.in b/src/daemon/abrt-handle-upload.in

index dbc4534..96e68b9 100755

--- a/src/daemon/abrt-handle-upload.in

+++ b/src/daemon/abrt-handle-upload.in

@@ -10,6 +10,7 @@ import getopt

 import tempfile

 import shutil

 import datetime

+import grp

 from reportclient import set_verbosity, error_msg_and_die, error_msg, log

@@ -36,12 +37,77 @@ def init_gettext():

 import problem

-def write_str_to(filename, s):

-    fd = os.open(filename, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, @DEFAULT_DUMP_DIR_MODE@ | stat.S_IROTH)

+def write_str_to(filename, s, uid, gid, mode):

+    fd = os.open(filename, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, mode)

     if fd >= 0:

+        os.fchown(fd, uid, gid)

         os.write(fd, s)

         os.close(fd)

+

+def validate_transform_move_and_notify(uploaded_dir_path, problem_dir_path, dest=None):

+    fsuid = 0

+    fsgid = 0

+

+    try:

+        gabrt = grp.getgrnam("abrt")

+        fsgid = gabrt.gr_gid

+    except KeyError as ex:

+        error_msg("Failed to get GID of 'abrt' (using 0 instead): {0}'".format(str(ex)))

+

+    try:

+        # give the uploaded directory to 'root:abrt' or 'root:root'

+        os.chown(uploaded_dir_path, fsuid, fsgid)

+        # set the right permissions for this machine

+        # (allow the owner and the group to access problem elements,

+        #  the default dump dir mode lacks x bit for both)

+        os.chmod(uploaded_dir_path, @DEFAULT_DUMP_DIR_MODE@ | stat.S_IXUSR | stat.S_IXGRP)

+

+        # sanitize problem elements

+        for item in os.listdir(uploaded_dir_path):

+            apath = os.path.join(uploaded_dir_path, item)

+            if os.path.islink(apath):

+                # remove symbolic links

+                os.remove(apath)

+            elif os.path.isdir(apath):

+                # remove directories

+                shutil.rmtree(apath)

+            elif os.path.isfile(apath):

+                # set file ownership to 'root:abrt' or 'root:root'

+                os.chown(apath, fsuid, fsgid)

+                # set the right file permissions for this machine

+                os.chmod(apath, @DEFAULT_DUMP_DIR_MODE@)

+            else:

+                # remove things that are neither files, symlinks nor directories

+                os.remove(apath)

+    except OSError as ex:

+        error_msg("Removing uploaded dir '{0}': '{1}'".format(uploaded_dir_path, str(ex)))

+        try:

+            shutil.rmtree(uploaded_dir_path)

+        except OSError as ex2:

+            error_msg_and_die("Failed to clean up dir '{0}': '{1}'".format(uploaded_dir_path, str(ex2)))

+        return

+

+    # overwrite remote if it exists

+    remote_path = os.path.join(uploaded_dir_path, "remote")

+    write_str_to(remote_path, "1", fsuid, fsgid, @DEFAULT_DUMP_DIR_MODE@)

+

+    # abrtd would increment count value and abrt-server refuses to process

+    # problem directories containing 'count' element when PrivateReports is on.

+    count_path = os.path.join(uploaded_dir_path, "count")

+    if os.path.exists(count_path):

+        # overwrite remote_count if it exists

+        remote_count_path = os.path.join(uploaded_dir_path, "remote_count")

+        os.rename(count_path, remote_count_path)

+

+    if not dest:

+        dest = problem_dir_path

+

+    shutil.move(uploaded_dir_path, dest)

+

+    problem.notify_new_path(problem_dir_path)

+

+

 if __name__ == "__main__":

     # Helper: exit with cleanup

@@ -176,22 +242,18 @@ if __name__ == "__main__":

         # The archive can contain either plain dump files

         # or one or more complete problem data directories.

         # Checking second possibility first.

-        if os.path.exists(tempdir+"/analyzer") and os.path.exists(tempdir+"/time"):

-            write_str_to(tempdir+"/remote", "1")

-            shutil.move(tempdir, abrt_dir)

-            problem.notify_new_path(abrt_dir+"/"+os.path.basename(tempdir))

+        if (os.path.exists(tempdir+"/analyzer") or os.path.exists(tempdir+"/type")) and os.path.exists(tempdir+"/time"):

+            validate_transform_move_and_notify(tempdir, abrt_dir+"/"+os.path.basename(tempdir), dest=abrt_dir)

         else:

             for d in os.listdir(tempdir):

                 if not os.path.isdir(tempdir+"/"+d):

                     continue

-                write_str_to(tempdir+"/"+d+"/remote", "1")

                 dst = abrt_dir+"/"+d

                 if os.path.exists(dst):

                     dst += "."+str(os.getpid())

                 if os.path.exists(dst):

                     continue

-                shutil.move(tempdir+"/"+d, dst)

-                problem.notify_new_path(dst)

+                validate_transform_move_and_notify(tempdir+"/"+d, dst)

         die_exitcode = 0

         # This deletes working_dir (== delete_on_exit)

-- 

2.1.0