abrt-hook-python: add input sanitization and directory size guard

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

5 files changed, 174 insertions, 95 deletions

diff --git a/src/Hooks/CCpp.cpp b/src/Hooks/CCpp.cpp
index 3a133588..4935b347 100644
--- a/src/Hooks/CCpp.cpp
+++ b/src/Hooks/CCpp.cpp
@@ -19,9 +19,11 @@
     51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 */
 #include "abrtlib.h"
+#include "hooklib.h"
 #include "DebugDump.h"
 #include "ABRTException.h"
 #include <syslog.h>
+#include <sys/statvfs.h>
 
 #define FILENAME_EXECUTABLE     "executable"
 #define FILENAME_COREDUMP       "coredump"
@@ -63,9 +65,9 @@ int main(int argc, char** argv)
     int fd;
     struct stat sb;
 
-    const char* program_name = argv[0];
     if (argc < 5)
     {
+        const char* program_name = argv[0];
         error_msg_and_die("Usage: %s: DUMPDIR PID SIGNO UID CORE_SIZE_LIMIT", program_name);
     }
     openlog("abrt", 0, LOG_DAEMON);
@@ -124,62 +126,13 @@ int main(int argc, char** argv)
         /* Parse abrt.conf and plugins/CCpp.conf */
         unsigned setting_MaxCrashReportsSize = 0;
         bool setting_MakeCompatCore = false;
-        bool abrt_conf = true;
-        FILE *fp = fopen(CONF_DIR"/abrt.conf", "r");
-        if (fp)
-        {
-            char line[256];
-            while (1)
-            {
-                if (fgets(line, sizeof(line), fp) == NULL)
-                {
-                    /* Next .conf file plz */
-                    if (abrt_conf)
-                    {
-                        abrt_conf = false;
-                        fp = fopen(CONF_DIR"/plugins/CCpp.conf", "r");
-                        if (fp)
-                            continue;
-                    }
-                    break;
-                }
+        parse_conf(CONF_DIR"/plugins/CCpp.conf", &setting_MaxCrashReportsSize, &setting_MakeCompatCore);
 
-                unsigned len = strlen(line);
-                if (len > 0 && line[len-1] == '\n')
-                    line[--len] = '\0';
-                const char *p = skip_whitespace(line);
-#undef DIRECTIVE
-#define DIRECTIVE "MaxCrashReportsSize"
-                if (strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)
-                {
-                    p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
-                    if (*p != '=')
-                        continue;
-                    p = skip_whitespace(p + 1);
-                    if (isdigit(*p))
-                        /* x1.25: go a bit up, so that usual in-daemon trimming
-                         * kicks in first, and we don't "fight" with it. */
-                        setting_MaxCrashReportsSize = (unsigned long)xatou(p) * 5 / 4;
-                    continue;
-                }
-#undef DIRECTIVE
-#define DIRECTIVE "MakeCompatCore"
-                if (strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)
-                {
-                    p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
-                    if (*p != '=')
-                        continue;
-                    p = skip_whitespace(p + 1);
-                    setting_MakeCompatCore = string_to_bool(p);
-                    continue;
-                }
-#undef DIRECTIVE
-                /* add more 'if (strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)' here... */
-            }
-            fclose(fp);
+        if (setting_MaxCrashReportsSize > 0)
+        {
+            check_free_space(setting_MaxCrashReportsSize);
         }
 
-
         char path[PATH_MAX];
 
         /* Check /var/cache/abrt/last-ccpp marker, do not dump repeated crashes
@@ -288,30 +241,10 @@ int main(int argc, char** argv)
          */
         dd.Close();
 
-        /* rhbz#539551: "abrt going crazy when crashing process is respawned".
-         * Do we want to protect against or ameliorate this? How? Ideas:
-         * (1) nice ourself?
-         * (2) check total size of dump dir, if it overflows, either abort dump
-         *     or delete oldest/biggest dumps? [abort would be simpler,
-         *     abrtd already does "delete on overflow" thing]
-         * (3) detect parallel dumps in progress and back off
-         *     (pause/renice further/??)
-         */
-
+        /* rhbz#539551: "abrt going crazy when crashing process is respawned" */
         if (setting_MaxCrashReportsSize > 0)
         {
-            string worst_dir;
-            while (1)
-            {
-                const char *base_dirname = strrchr(path, '/') + 1; /* never NULL */
-                /* We exclude our own dump from candidates for deletion (3rd param): */
-                double dirsize = get_dirsize_find_largest_dir(DEBUG_DUMPS_DIR, &worst_dir, base_dirname);
-                if (dirsize / (1024*1024) < setting_MaxCrashReportsSize || worst_dir == "")
-                    break;
-                log("size of '%s' >= %u MB, deleting '%s'", DEBUG_DUMPS_DIR, setting_MaxCrashReportsSize, worst_dir.c_str());
-                delete_debug_dump_dir(concat_path_file(DEBUG_DUMPS_DIR, worst_dir.c_str()).c_str());
-                worst_dir = "";
-            }
+            trim_debug_dumps(setting_MaxCrashReportsSize, path);
         }
 
         if (!setting_MakeCompatCore)
diff --git a/src/Hooks/Makefile.am b/src/Hooks/Makefile.am
index 75819719..ebabd87b 100644
--- a/src/Hooks/Makefile.am
+++ b/src/Hooks/Makefile.am
@@ -1,9 +1,10 @@
 libexec_PROGRAMS = abrt-hook-ccpp abrt-hook-python
 bin_PROGRAMS = dumpoops
 
-# CCpp
+# abrt-hook-ccpp
 abrt_hook_ccpp_SOURCES = \
-    CCpp.cpp
+    CCpp.cpp \
+    hooklib.h hooklib.cpp
 abrt_hook_ccpp_CPPFLAGS = \
     -I$(srcdir)/../../inc \
     -I$(srcdir)/../../lib/Utils \
@@ -34,7 +35,8 @@ dumpoops_LDADD = \
 
 # abrt-hook-python
 abrt_hook_python_SOURCES = \
-    abrt-hook-python.cpp
+    abrt-hook-python.cpp \
+    hooklib.h hooklib.cpp
 abrt_hook_python_CPPFLAGS = \
     -I$(srcdir)/../../inc \
     -I$(srcdir)/../../lib/Utils \
diff --git a/src/Hooks/abrt-hook-python.cpp b/src/Hooks/abrt-hook-python.cpp
index 3f79d28a..406cd829 100644
--- a/src/Hooks/abrt-hook-python.cpp
+++ b/src/Hooks/abrt-hook-python.cpp
@@ -24,12 +24,14 @@
 /* We can easily get rid of abrtlib (libABRTUtils.so) usage in this file,
  * but DebugDump will pull it in anyway */
 #include "abrtlib.h"
+#include "hooklib.h"
 #include "DebugDump.h"
 #if HAVE_CONFIG_H
 # include <config.h>
 #endif
 
 #define MAX_BT_SIZE (1024*1024)
+#define MAX_BT_SIZE_STR "1 MB"
 
 static char *pid;
 static char *executable;
@@ -74,9 +76,15 @@ int main(int argc, char** argv)
       );
     }
   }
-  if (!pid)
+  if (!pid || !executable || !uuid)
     goto usage;
-// is it really ok if other params aren't specified? abrtd might get confused...
+
+  unsigned setting_MaxCrashReportsSize = 0;
+  parse_conf(NULL, &setting_MaxCrashReportsSize, NULL);
+  if (setting_MaxCrashReportsSize > 0)
+  {
+    check_free_space(setting_MaxCrashReportsSize);
+  }
 
   // Read the backtrace from stdin
   char *bt = (char*)xmalloc(MAX_BT_SIZE);
@@ -88,35 +96,34 @@ int main(int argc, char** argv)
   bt[len] = '\0';
   if (len == MAX_BT_SIZE-1)
   {
-    error_msg("Backtrace size limit exceeded, trimming to 1 MB");
+    error_msg("Backtrace size limit exceeded, trimming to " MAX_BT_SIZE_STR);
   }
 
+  char *cmdline = get_cmdline(xatou(pid)); /* never NULL */
+
   // Create directory with the debug dump
   char path[PATH_MAX];
   snprintf(path, sizeof(path), DEBUG_DUMPS_DIR"/pyhook-%ld-%s",
 	   (long)time(NULL), pid);
-
   CDebugDump dd;
   dd.Create(path, geteuid());
-  dd.SaveText(FILENAME_ANALYZER, "Python");
-  if (executable)
-    dd.SaveText(FILENAME_EXECUTABLE, executable);
 
-  pid_t pidt = xatoi(pid);
-  char *cmdline = get_cmdline(pidt);
+  dd.SaveText(FILENAME_ANALYZER, "Python");
+  dd.SaveText(FILENAME_EXECUTABLE, executable);
+  dd.SaveText("backtrace", bt);
+  free(bt);
   dd.SaveText("cmdline", cmdline);
   free(cmdline);
-
-  if (uuid)
-    dd.SaveText("uuid", uuid);
-
+  dd.SaveText("uuid", uuid);
   char uid[sizeof(int) * 3 + 2];
-  sprintf(uid, "%d", (int)getuid());
+  sprintf(uid, "%u", (unsigned)getuid());
   dd.SaveText("uid", uid);
 
-  dd.SaveText("backtrace", bt);
-  free(bt);
   dd.Close();
+  if (setting_MaxCrashReportsSize > 0)
+  {
+    trim_debug_dumps(setting_MaxCrashReportsSize, path);
+  }
 
   return 0;
 }
diff --git a/src/Hooks/hooklib.cpp b/src/Hooks/hooklib.cpp
new file mode 100644
index 00000000..41b9627f
--- /dev/null
+++ b/src/Hooks/hooklib.cpp
@@ -0,0 +1,116 @@
+/*
+    Copyright (C) 2009	RedHat inc.
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License along
+    with this program; if not, write to the Free Software Foundation, Inc.,
+    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+*/
+#include "abrtlib.h"
+#include "hooklib.h"
+#include "DebugDump.h"
+#include <sys/statvfs.h>
+
+using namespace std;
+
+void parse_conf(const char *additional_conf, unsigned *setting_MaxCrashReportsSize, bool *setting_MakeCompatCore)
+{
+    FILE *fp = fopen(CONF_DIR"/abrt.conf", "r");
+    if (!fp)
+        return;
+
+    char line[256];
+    while (1)
+    {
+        if (fgets(line, sizeof(line), fp) == NULL)
+        {
+            fclose(fp);
+            if (additional_conf)
+            {
+                /* Next .conf file plz */
+                fp = fopen(additional_conf, "r");
+                if (fp)
+                {
+                    additional_conf = NULL;
+                    continue;
+                }
+            }
+            break;
+        }
+
+        unsigned len = strlen(line);
+        if (len > 0 && line[len-1] == '\n')
+            line[--len] = '\0';
+        const char *p = skip_whitespace(line);
+#undef DIRECTIVE
+#define DIRECTIVE "MaxCrashReportsSize"
+        if (setting_MaxCrashReportsSize && strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)
+        {
+            p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
+            if (*p != '=')
+                continue;
+            p = skip_whitespace(p + 1);
+            if (isdigit(*p))
+            {
+                /* x1.25: go a bit up, so that usual in-daemon trimming
+                 * kicks in first, and we don't "fight" with it. */
+                *setting_MaxCrashReportsSize = (unsigned long)xatou(p) * 5 / 4;
+            }
+            continue;
+        }
+#undef DIRECTIVE
+#define DIRECTIVE "MakeCompatCore"
+        if (setting_MakeCompatCore && strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)
+        {
+            p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
+            if (*p != '=')
+                continue;
+            p = skip_whitespace(p + 1);
+            *setting_MakeCompatCore = string_to_bool(p);
+            continue;
+        }
+#undef DIRECTIVE
+        /* add more 'if (strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)' here... */
+    }
+}
+
+void check_free_space(unsigned setting_MaxCrashReportsSize)
+{
+    /* Check that at least MaxCrashReportsSize/4 MBs are free. */
+    struct statvfs vfs;
+    if (statvfs(DEBUG_DUMPS_DIR, &vfs) != 0
+     || (vfs.f_bfree / (1024*1024 / 4)) * vfs.f_bsize < setting_MaxCrashReportsSize
+    ) {
+        error_msg_and_die("Low free disk space detected, aborting dump");
+    }
+}
+
+/* rhbz#539551: "abrt going crazy when crashing process is respawned".
+ * Check total size of dump dir, if it overflows,
+ * delete oldest/biggest dumps.
+ */
+void trim_debug_dumps(unsigned setting_MaxCrashReportsSize, const char *exclude_path)
+{
+    int count = 10;
+    string worst_dir;
+    while (--count >= 0)
+    {
+        const char *base_dirname = strrchr(exclude_path, '/') + 1; /* never NULL */
+        /* We exclude our own dump from candidates for deletion (3rd param): */
+        double dirsize = get_dirsize_find_largest_dir(DEBUG_DUMPS_DIR, &worst_dir, base_dirname);
+        if (dirsize / (1024*1024) < setting_MaxCrashReportsSize || worst_dir == "")
+            break;
+        log("size of '%s' >= %u MB, deleting '%s'", DEBUG_DUMPS_DIR, setting_MaxCrashReportsSize, worst_dir.c_str());
+        delete_debug_dump_dir(concat_path_file(DEBUG_DUMPS_DIR, worst_dir.c_str()).c_str());
+        worst_dir = "";
+    }
+}
diff --git a/src/Hooks/hooklib.h b/src/Hooks/hooklib.h
new file mode 100644
index 00000000..0794ff60
--- /dev/null
+++ b/src/Hooks/hooklib.h
@@ -0,0 +1,21 @@
+/*
+    Copyright (C) 2009	RedHat inc.
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License along
+    with this program; if not, write to the Free Software Foundation, Inc.,
+    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+*/
+
+void parse_conf(const char *additional_conf, unsigned *setting_MaxCrashReportsSize, bool *setting_MakeCompatCore);
+void check_free_space(unsigned setting_MaxCrashReportsSize);
+void trim_debug_dumps(unsigned setting_MaxCrashReportsSize, const char *exclude_path);