File _service:download_src_package:0110-a-a-i-d-t-a-... of Package abrt

Overview Repositories Revisions Requests Users Attributes Meta

File _service:download_src_package:0110-a-a-i-d-t-a-cache-sanitize-arguments.patch of Package abrt

From 9943a77bca37a0829ccd3784d1dfab37f8c24e7b Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Wed, 29 Apr 2015 13:57:39 +0200
Subject: [ABRT PATCH] a-a-i-d-t-a-cache: sanitize arguments

Parse command lines arguments and use them to create new arguments for
exec(). No black listing algorithm would be safe enough. The only
allowed arguments are the following:
* v - verbose
* y - noninteractive
* repo - enable only repositories whose names match the pattern
* exact - download packages for the specified files
* ids - passed as magic proc fd path to the wrapped executable

The wrapper opens the list of needed build ids passes /proc/self/fd/[fd]
to the wrapped process. This allows us to open the file with caller's
UID/GID in order to avoid information disclosures.

Forbidden arguments:
* cache - allows regular users to create a user writable dump directory
* tmpdir - the same as above
* size_mb - no need to allow users to fiddle with the cache size

Related: #1216962

Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
 po/POTFILES.in                                     |   1 +
 .../abrt-action-install-debuginfo-to-abrt-cache.c  | 106 ++++++++++++++++-----
 2 files changed, 85 insertions(+), 22 deletions(-)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index cbe89fa..1248c46 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -31,6 +31,7 @@ src/plugins/abrt-action-check-oops-for-hw-error.in
 src/plugins/abrt-action-generate-backtrace.c
 src/plugins/abrt-action-generate-core-backtrace.c
 src/plugins/abrt-action-install-debuginfo.in
+src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
 src/plugins/abrt-action-perform-ccpp-analysis.in
 src/plugins/abrt-action-trim-files.c
 src/plugins/abrt-action-ureport
diff --git a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
index e0eccc0..eb2f7c5 100644
--- a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
+++ b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
@@ -29,28 +29,90 @@
  */
 int main(int argc, char **argv)
 {
-    /*
-     * We disallow passing of arguments which point to writable dirs
-     * and other files possibly not accessible to calling user.
-     * This way, the script will always use default values for these arguments.
-     */
-    char **pp = argv;
-    char *arg;
-    while ((arg = *++pp) != NULL)
+    /* I18n */
+    setlocale(LC_ALL, "");
+#if ENABLE_NLS
+    bindtextdomain(PACKAGE, LOCALEDIR);
+    textdomain(PACKAGE);
+#endif
+
+    abrt_init(argv);
+
+    /* Can't keep these strings/structs static: _() doesn't support that */
+    const char *program_usage_string = _(
+        "& [-y] [-i BUILD_IDS_FILE|-i -] [-e PATH[:PATH]...]\n"
+        "\t[-r REPO]\n"
+        "\n"
+        "Installs debuginfo packages for all build-ids listed in BUILD_IDS_FILE to\n"
+        "ABRT system cache."
+    );
+
+    enum {
+        OPT_v = 1 << 0,
+        OPT_y = 1 << 1,
+        OPT_i = 1 << 2,
+        OPT_e = 1 << 3,
+        OPT_r = 1 << 4,
+        OPT_s = 1 << 5,
+    };
+
+    const char *build_ids = "build_ids";
+    const char *exact = NULL;
+    const char *repo = NULL;
+    const char *size_mb = NULL;
+
+    struct options program_options[] = {
+        OPT__VERBOSE(&g_verbose),
+        OPT_BOOL  ('y', "yes",         NULL,                   _("Noninteractive, assume 'Yes' to all questions")),
+        OPT_STRING('i', "ids",   &build_ids, "BUILD_IDS_FILE", _("- means STDIN, default: build_ids")),
+        OPT_STRING('e', "exact",     &exact, "EXACT",          _("Download only specified files")),
+        OPT_STRING('r', "repo",       &repo, "REPO",           _("Pattern to use when searching for repos, default: *debug*")),
+        OPT_STRING('s', "size_mb", &size_mb, "SIZE_MB",        _("Ignored option")),
+        OPT_END()
+    };
+    const unsigned opts = parse_opts(argc, argv, program_options, program_usage_string);
+
+    /* We need to open the build ids file under the caller's UID/GID to avoid
+     * information disclosures when reading files with changed UID.
+     * Unfortunately, we cannot replace STDIN with the new fd because ABRT uses
+     * STDIN to communicate with the caller. So, the following code opens a
+     * dummy file descriptor to the build ids file and passes the new fd's proc
+     * path to the wrapped program in the ids argument.
+     * The new fd remains opened, the OS will close it for us. */
+    char *build_ids_self_fd = NULL;
+    if (strcmp("-", build_ids) != 0)
+    {
+        const int build_ids_fd = open(build_ids, O_RDONLY);
+        if (build_ids_fd < 0)
+            perror_msg_and_die("Failed to open file '%s'", build_ids);
+
+        /* We are not going to free this memory. There is no place to do so. */
+        build_ids_self_fd = xasprintf("/proc/self/fd/%d", build_ids_fd);
+    }
+
+    /* name, -v, --ids, -, -y, -e, EXACT, -r, REPO, --, NULL */
+    const char *args[11];
     {
-        /* Allow taking ids from stdin */
-        if (strcmp(arg, "--ids=-") == 0)
-            continue;
-
-        if (strncmp(arg, "--exact", 7) == 0)
-            continue;
-
-        if (strncmp(arg, "--cache", 7) == 0)
-            error_msg_and_die("bad option %s", arg);
-        if (strncmp(arg, "--tmpdir", 8) == 0)
-            error_msg_and_die("bad option %s", arg);
-        if (strncmp(arg, "--ids", 5) == 0)
-            error_msg_and_die("bad option %s", arg);
+        const char *verbs[] = { "", "-v", "-vv", "-vvv" };
+        unsigned i = 0;
+        args[i++] = EXECUTABLE;
+        args[i++] = "--ids";
+        args[i++] = (build_ids_self_fd != NULL) ? build_ids_self_fd : "-";
+        args[i++] = verbs[g_verbose <= 3 ? g_verbose : 3];
+        if ((opts & OPT_y))
+            args[i++] = "-y";
+        if ((opts & OPT_e))
+        {
+            args[i++] = "--exact";
+            args[i++] = exact;
+        }
+        if ((opts & OPT_r))
+        {
+            args[i++] = "--repo";
+            args[i++] = repo;
+        }
+        args[i++] = "--";
+        args[i] = NULL;
     }
 
     /* Switch real user/group to effective ones.
@@ -122,6 +184,6 @@ int main(int argc, char **argv)
         putenv(path_env);
     }
 
-    execvp(EXECUTABLE, argv);
+    execvp(EXECUTABLE, (char **)args);
     error_msg_and_die("Can't execute %s", EXECUTABLE);
 }
-- 
1.8.3.1

Places

File _service:download_src_package:0110-a-a-i-d-t-a-cache-sanitize-arguments.patch of Package abrt

Places