doczkal / rpms / abrt

Forked from rpms/abrt 4 years ago
Clone

Blame SOURCES/0113-upload-validate-and-sanitize-uploaded-dump-directori.patch

a60cd7
From a4e47c753e9d7988f4d938ed2e0fd690a909ce68 Mon Sep 17 00:00:00 2001
a60cd7
From: Jakub Filak <jfilak@redhat.com>
a60cd7
Date: Mon, 20 Apr 2015 15:15:40 +0200
a60cd7
Subject: [ABRT PATCH] upload: validate and sanitize uploaded dump directories
a60cd7
a60cd7
It was discovered that, when moving problem reports from
a60cd7
/var/spool/abrt-upload to /var/spool/abrt or /var/tmp/abrt,
a60cd7
abrt-handle-upload does not verify that the new problem directory
a60cd7
has appropriate permissions and does not contain symbolic links.  A
a60cd7
crafted problem report exposes other parts of abrt to attack, and
a60cd7
the abrt-handle-upload script allows to overwrite arbitrary files.
a60cd7
a60cd7
Acknowledgement:
a60cd7
a60cd7
This issue was discovered by Florian Weimer of Red Hat Product Security.
a60cd7
a60cd7
Related: #1212953
a60cd7
a60cd7
Signed-off-by: Jakub Filak <jfilak@redhat.com>
a60cd7
---
a60cd7
 src/daemon/abrt-handle-upload.in | 78 +++++++++++++++++++++++++++++++++++-----
a60cd7
 1 file changed, 70 insertions(+), 8 deletions(-)
a60cd7
a60cd7
diff --git a/src/daemon/abrt-handle-upload.in b/src/daemon/abrt-handle-upload.in
a60cd7
index dbc4534..7720da4 100755
a60cd7
--- a/src/daemon/abrt-handle-upload.in
a60cd7
+++ b/src/daemon/abrt-handle-upload.in
a60cd7
@@ -10,6 +10,7 @@ import getopt
a60cd7
 import tempfile
a60cd7
 import shutil
a60cd7
 import datetime
a60cd7
+import grp
a60cd7
 
a60cd7
 from reportclient import set_verbosity, error_msg_and_die, error_msg, log
a60cd7
 
a60cd7
@@ -36,12 +37,77 @@ def init_gettext():
a60cd7
 
a60cd7
 import problem
a60cd7
 
a60cd7
-def write_str_to(filename, s):
a60cd7
-    fd = os.open(filename, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, @DEFAULT_DUMP_DIR_MODE@ | stat.S_IROTH)
a60cd7
+def write_str_to(filename, s, uid, gid, mode):
a60cd7
+    fd = os.open(filename, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, mode)
a60cd7
     if fd >= 0:
a60cd7
+        os.fchown(fd, uid, gid)
a60cd7
         os.write(fd, s)
a60cd7
         os.close(fd)
a60cd7
 
a60cd7
+
a60cd7
+def validate_transform_move_and_notify(uploaded_dir_path, problem_dir_path, dest=None):
a60cd7
+    fsuid = 0
a60cd7
+    fsgid = 0
a60cd7
+
a60cd7
+    try:
a60cd7
+        gabrt = grp.getgrnam("abrt")
a60cd7
+        fsgid = gabrt.gr_gid
a60cd7
+    except KeyError as ex:
a60cd7
+        error_msg("Failed to get GID of 'abrt' (using 0 instead): {0}'".format(str(ex)))
a60cd7
+
a60cd7
+    try:
a60cd7
+        # give the uploaded directory to 'root:abrt' or 'root:root'
a60cd7
+        os.chown(uploaded_dir_path, fsuid, fsgid)
a60cd7
+        # set the right permissions for this machine
a60cd7
+        # (allow the owner and the group to access problem elements,
a60cd7
+        #  the default dump dir mode lacks x bit for both)
a60cd7
+        os.chmod(uploaded_dir_path, @DEFAULT_DUMP_DIR_MODE@ | stat.S_IXUSR | stat.S_IXGRP)
a60cd7
+
a60cd7
+        # sanitize problem elements
a60cd7
+        for item in os.listdir(uploaded_dir_path):
a60cd7
+            apath = os.path.join(uploaded_dir_path, item)
a60cd7
+            if os.path.islink(apath):
a60cd7
+                # remove symbolic links
a60cd7
+                os.remove(apath)
a60cd7
+            elif os.path.isdir(apath):
a60cd7
+                # remove directories
a60cd7
+                shutil.rmtree(apath)
a60cd7
+            elif os.path.isfile(apath):
a60cd7
+                # set file ownership to 'root:abrt' or 'root:root'
a60cd7
+                os.chown(apath, fsuid, fsgid)
a60cd7
+                # set the right file permissions for this machine
a60cd7
+                os.chmod(apath, @DEFAULT_DUMP_DIR_MODE@)
a60cd7
+            else:
a60cd7
+                # remove things that are neither files, symlinks nor directories
a60cd7
+                os.remove(apath)
a60cd7
+    except OSError as ex:
a60cd7
+        error_msg("Removing uploaded dir '{0}': '{1}'".format(uploaded_dir_path, str(ex)))
a60cd7
+        try:
a60cd7
+            shutil.rmtree(uploaded_dir_path)
a60cd7
+        except OSError as ex2:
a60cd7
+            error_msg_and_die("Failed to clean up dir '{0}': '{1}'".format(uploaded_dir_path, str(ex2)))
a60cd7
+        return
a60cd7
+
a60cd7
+    # overwrite remote if it exists
a60cd7
+    remote_path = os.path.join(uploaded_dir_path, "remote")
a60cd7
+    write_str_to(remote_path, "1", fsuid, fsgid, @DEFAULT_DUMP_DIR_MODE@)
a60cd7
+
a60cd7
+    # abrtd would increment count value and abrt-server refuses to process
a60cd7
+    # problem directories containing 'count' element when PrivateReports is on.
a60cd7
+    count_path = os.path.join(uploaded_dir_path, "count")
a60cd7
+    if os.path.exists(count_path):
a60cd7
+        # overwrite remote_count if it exists
a60cd7
+        remote_count_path = os.path.join(uploaded_dir_path, "remote_count")
a60cd7
+        os.rename(count_path, remote_count_path)
a60cd7
+
a60cd7
+    if not dest:
a60cd7
+        dest = problem_dir_path
a60cd7
+
a60cd7
+    shutil.move(uploaded_dir_path, dest)
a60cd7
+
a60cd7
+    problem.notify_new_path(problem_dir_path)
a60cd7
+
a60cd7
+
a60cd7
 if __name__ == "__main__":
a60cd7
 
a60cd7
     # Helper: exit with cleanup
a60cd7
@@ -177,21 +243,17 @@ if __name__ == "__main__":
a60cd7
         # or one or more complete problem data directories.
a60cd7
         # Checking second possibility first.
a60cd7
         if os.path.exists(tempdir+"/analyzer") and os.path.exists(tempdir+"/time"):
a60cd7
-            write_str_to(tempdir+"/remote", "1")
a60cd7
-            shutil.move(tempdir, abrt_dir)
a60cd7
-            problem.notify_new_path(abrt_dir+"/"+os.path.basename(tempdir))
a60cd7
+            validate_transform_move_and_notify(tempdir, abrt_dir+"/"+os.path.basename(tempdir), dest=abrt_dir)
a60cd7
         else:
a60cd7
             for d in os.listdir(tempdir):
a60cd7
                 if not os.path.isdir(tempdir+"/"+d):
a60cd7
                     continue
a60cd7
-                write_str_to(tempdir+"/"+d+"/remote", "1")
a60cd7
                 dst = abrt_dir+"/"+d
a60cd7
                 if os.path.exists(dst):
a60cd7
                     dst += "."+str(os.getpid())
a60cd7
                 if os.path.exists(dst):
a60cd7
                     continue
a60cd7
-                shutil.move(tempdir+"/"+d, dst)
a60cd7
-                problem.notify_new_path(dst)
a60cd7
+                validate_transform_move_and_notify(tempdir+"/"+d, dst)
a60cd7
 
a60cd7
         die_exitcode = 0
a60cd7
         # This deletes working_dir (== delete_on_exit)
a60cd7
-- 
a60cd7
1.8.3.1
a60cd7