8ec399
From 9943a77bca37a0829ccd3784d1dfab37f8c24e7b Mon Sep 17 00:00:00 2001
8ec399
From: Jakub Filak <jfilak@redhat.com>
8ec399
Date: Wed, 29 Apr 2015 13:57:39 +0200
8ec399
Subject: [ABRT PATCH] a-a-i-d-t-a-cache: sanitize arguments
8ec399
8ec399
Parse command lines arguments and use them to create new arguments for
8ec399
exec(). No black listing algorithm would be safe enough. The only
8ec399
allowed arguments are the following:
8ec399
* v - verbose
8ec399
* y - noninteractive
8ec399
* repo - enable only repositories whose names match the pattern
8ec399
* exact - download packages for the specified files
8ec399
* ids - passed as magic proc fd path to the wrapped executable
8ec399
8ec399
The wrapper opens the list of needed build ids passes /proc/self/fd/[fd]
8ec399
to the wrapped process. This allows us to open the file with caller's
8ec399
UID/GID in order to avoid information disclosures.
8ec399
8ec399
Forbidden arguments:
8ec399
* cache - allows regular users to create a user writable dump directory
8ec399
* tmpdir - the same as above
8ec399
* size_mb - no need to allow users to fiddle with the cache size
8ec399
8ec399
Related: #1216962
8ec399
8ec399
Signed-off-by: Jakub Filak <jfilak@redhat.com>
8ec399
---
8ec399
 po/POTFILES.in                                     |   1 +
8ec399
 .../abrt-action-install-debuginfo-to-abrt-cache.c  | 106 ++++++++++++++++-----
8ec399
 2 files changed, 85 insertions(+), 22 deletions(-)
8ec399
8ec399
diff --git a/po/POTFILES.in b/po/POTFILES.in
8ec399
index cbe89fa..1248c46 100644
8ec399
--- a/po/POTFILES.in
8ec399
+++ b/po/POTFILES.in
8ec399
@@ -31,6 +31,7 @@ src/plugins/abrt-action-check-oops-for-hw-error.in
8ec399
 src/plugins/abrt-action-generate-backtrace.c
8ec399
 src/plugins/abrt-action-generate-core-backtrace.c
8ec399
 src/plugins/abrt-action-install-debuginfo.in
8ec399
+src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
8ec399
 src/plugins/abrt-action-perform-ccpp-analysis.in
8ec399
 src/plugins/abrt-action-trim-files.c
8ec399
 src/plugins/abrt-action-ureport
8ec399
diff --git a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
8ec399
index e0eccc0..eb2f7c5 100644
8ec399
--- a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
8ec399
+++ b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
8ec399
@@ -29,28 +29,90 @@
8ec399
  */
8ec399
 int main(int argc, char **argv)
8ec399
 {
8ec399
-    /*
8ec399
-     * We disallow passing of arguments which point to writable dirs
8ec399
-     * and other files possibly not accessible to calling user.
8ec399
-     * This way, the script will always use default values for these arguments.
8ec399
-     */
8ec399
-    char **pp = argv;
8ec399
-    char *arg;
8ec399
-    while ((arg = *++pp) != NULL)
8ec399
+    /* I18n */
8ec399
+    setlocale(LC_ALL, "");
8ec399
+#if ENABLE_NLS
8ec399
+    bindtextdomain(PACKAGE, LOCALEDIR);
8ec399
+    textdomain(PACKAGE);
8ec399
+#endif
8ec399
+
8ec399
+    abrt_init(argv);
8ec399
+
8ec399
+    /* Can't keep these strings/structs static: _() doesn't support that */
8ec399
+    const char *program_usage_string = _(
8ec399
+        "& [-y] [-i BUILD_IDS_FILE|-i -] [-e PATH[:PATH]...]\n"
8ec399
+        "\t[-r REPO]\n"
8ec399
+        "\n"
8ec399
+        "Installs debuginfo packages for all build-ids listed in BUILD_IDS_FILE to\n"
8ec399
+        "ABRT system cache."
8ec399
+    );
8ec399
+
8ec399
+    enum {
8ec399
+        OPT_v = 1 << 0,
8ec399
+        OPT_y = 1 << 1,
8ec399
+        OPT_i = 1 << 2,
8ec399
+        OPT_e = 1 << 3,
8ec399
+        OPT_r = 1 << 4,
8ec399
+        OPT_s = 1 << 5,
8ec399
+    };
8ec399
+
8ec399
+    const char *build_ids = "build_ids";
8ec399
+    const char *exact = NULL;
8ec399
+    const char *repo = NULL;
8ec399
+    const char *size_mb = NULL;
8ec399
+
8ec399
+    struct options program_options[] = {
8ec399
+        OPT__VERBOSE(&g_verbose),
8ec399
+        OPT_BOOL  ('y', "yes",         NULL,                   _("Noninteractive, assume 'Yes' to all questions")),
8ec399
+        OPT_STRING('i', "ids",   &build_ids, "BUILD_IDS_FILE", _("- means STDIN, default: build_ids")),
8ec399
+        OPT_STRING('e', "exact",     &exact, "EXACT",          _("Download only specified files")),
8ec399
+        OPT_STRING('r', "repo",       &repo, "REPO",           _("Pattern to use when searching for repos, default: *debug*")),
8ec399
+        OPT_STRING('s', "size_mb", &size_mb, "SIZE_MB",        _("Ignored option")),
8ec399
+        OPT_END()
8ec399
+    };
8ec399
+    const unsigned opts = parse_opts(argc, argv, program_options, program_usage_string);
8ec399
+
8ec399
+    /* We need to open the build ids file under the caller's UID/GID to avoid
8ec399
+     * information disclosures when reading files with changed UID.
8ec399
+     * Unfortunately, we cannot replace STDIN with the new fd because ABRT uses
8ec399
+     * STDIN to communicate with the caller. So, the following code opens a
8ec399
+     * dummy file descriptor to the build ids file and passes the new fd's proc
8ec399
+     * path to the wrapped program in the ids argument.
8ec399
+     * The new fd remains opened, the OS will close it for us. */
8ec399
+    char *build_ids_self_fd = NULL;
8ec399
+    if (strcmp("-", build_ids) != 0)
8ec399
+    {
8ec399
+        const int build_ids_fd = open(build_ids, O_RDONLY);
8ec399
+        if (build_ids_fd < 0)
8ec399
+            perror_msg_and_die("Failed to open file '%s'", build_ids);
8ec399
+
8ec399
+        /* We are not going to free this memory. There is no place to do so. */
8ec399
+        build_ids_self_fd = xasprintf("/proc/self/fd/%d", build_ids_fd);
8ec399
+    }
8ec399
+
8ec399
+    /* name, -v, --ids, -, -y, -e, EXACT, -r, REPO, --, NULL */
8ec399
+    const char *args[11];
8ec399
     {
8ec399
-        /* Allow taking ids from stdin */
8ec399
-        if (strcmp(arg, "--ids=-") == 0)
8ec399
-            continue;
8ec399
-
8ec399
-        if (strncmp(arg, "--exact", 7) == 0)
8ec399
-            continue;
8ec399
-
8ec399
-        if (strncmp(arg, "--cache", 7) == 0)
8ec399
-            error_msg_and_die("bad option %s", arg);
8ec399
-        if (strncmp(arg, "--tmpdir", 8) == 0)
8ec399
-            error_msg_and_die("bad option %s", arg);
8ec399
-        if (strncmp(arg, "--ids", 5) == 0)
8ec399
-            error_msg_and_die("bad option %s", arg);
8ec399
+        const char *verbs[] = { "", "-v", "-vv", "-vvv" };
8ec399
+        unsigned i = 0;
8ec399
+        args[i++] = EXECUTABLE;
8ec399
+        args[i++] = "--ids";
8ec399
+        args[i++] = (build_ids_self_fd != NULL) ? build_ids_self_fd : "-";
8ec399
+        args[i++] = verbs[g_verbose <= 3 ? g_verbose : 3];
8ec399
+        if ((opts & OPT_y))
8ec399
+            args[i++] = "-y";
8ec399
+        if ((opts & OPT_e))
8ec399
+        {
8ec399
+            args[i++] = "--exact";
8ec399
+            args[i++] = exact;
8ec399
+        }
8ec399
+        if ((opts & OPT_r))
8ec399
+        {
8ec399
+            args[i++] = "--repo";
8ec399
+            args[i++] = repo;
8ec399
+        }
8ec399
+        args[i++] = "--";
8ec399
+        args[i] = NULL;
8ec399
     }
8ec399
 
8ec399
     /* Switch real user/group to effective ones.
8ec399
@@ -122,6 +184,6 @@ int main(int argc, char **argv)
8ec399
         putenv(path_env);
8ec399
     }
8ec399
 
8ec399
-    execvp(EXECUTABLE, argv);
8ec399
+    execvp(EXECUTABLE, (char **)args);
8ec399
     error_msg_and_die("Can't execute %s", EXECUTABLE);
8ec399
 }
8ec399
-- 
8ec399
1.8.3.1
8ec399