abrt-hook-python: sanitize input more; log to syslog

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

2 files changed, 44 insertions, 12 deletions

diff --git a/src/Hooks/abrt-hook-ccpp.cpp b/src/Hooks/abrt-hook-ccpp.cpp
index 237ea6fb..1c91dc8a 100644
--- a/src/Hooks/abrt-hook-ccpp.cpp
+++ b/src/Hooks/abrt-hook-ccpp.cpp
@@ -23,7 +23,6 @@
 #include "DebugDump.h"
 #include "ABRTException.h"
 #include <syslog.h>
-#include <sys/statvfs.h>
 
 #define FILENAME_EXECUTABLE     "executable"
 #define FILENAME_COREDUMP       "coredump"
@@ -70,7 +69,7 @@ int main(int argc, char** argv)
         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_PID | LOG_DAEMON);
+    openlog("abrt", LOG_PID, LOG_DAEMON);
     logmode = LOGMODE_SYSLOG;
 
     errno = 0;
diff --git a/src/Hooks/abrt-hook-python.cpp b/src/Hooks/abrt-hook-python.cpp
index b921fba2..c8a25e39 100644
--- a/src/Hooks/abrt-hook-python.cpp
+++ b/src/Hooks/abrt-hook-python.cpp
@@ -20,7 +20,7 @@
 */
 
 #include <getopt.h>
-#include <unistd.h>
+#include <syslog.h>
 /* We can easily get rid of abrtlib (libABRTUtils.so) usage in this file,
  * but DebugDump will pull it in anyway */
 #include "abrtlib.h"
@@ -38,12 +38,33 @@ static char *pid;
 static char *executable;
 static char *uuid;
 
-int main(int argc, char** argv)
+/* Note: "" will return false */
+static bool isxdigit_str(const char *str)
 {
-  // Error if daemon is not running.
-  if (!daemon_is_ok())
-    error_msg_and_die("Daemon is not running.");
+  do {
+    if ((*str < '0' || *str > '9') /* not a digit */
+     && ((*str | 0x20) < 'a' || (*str | 0x20) > 'f') /* not A-F or a-f */
+    )
+    {
+      return false;
+    }
+    str++;
+  } while (*str);
+  return true;
+}
+
+static bool printable_str(const char *str)
+{
+  do {
+    if ((unsigned char)(*str) < ' ' || *str == 0x7f)
+      return false;
+    str++;
+  } while (*str);
+  return true;
+}
 
+int main(int argc, char** argv)
+{
   // Parse options
   static const struct option longopts[] = {
     // name       , has_arg          , flag, val
@@ -79,8 +100,18 @@ int main(int argc, char** argv)
   }
   if (!pid || !executable || !uuid)
     goto usage;
+  if (strlen(uuid) > 128 || !isxdigit_str(uuid))
+    goto usage;
+  if (strlen(executable) > PATH_MAX || !printable_str(executable))
+    goto usage;
+  // pid string is sanitized later by xatou()
 
-//TODO: sanitize uuid and executable (size, valid chars etc)
+  openlog("abrt", LOG_PID, LOG_DAEMON);
+  logmode = LOGMODE_SYSLOG;
+
+  // Error if daemon is not running
+  if (!daemon_is_ok())
+    error_msg_and_die("daemon is not running, python crash dump aborted");
 
   unsigned setting_MaxCrashReportsSize = 0;
   parse_conf(NULL, &setting_MaxCrashReportsSize, NULL);
@@ -94,14 +125,15 @@ int main(int argc, char** argv)
   ssize_t len = full_read(STDIN_FILENO, bt, MAX_BT_SIZE-1);
   if (len < 0)
   {
-    perror_msg_and_die("Read error");
+    perror_msg_and_die("read error");
   }
   bt[len] = '\0';
   if (len == MAX_BT_SIZE-1)
   {
-    error_msg("Backtrace size limit exceeded, trimming to " MAX_BT_SIZE_STR);
+    error_msg("backtrace size limit exceeded, trimming to " MAX_BT_SIZE_STR);
   }
 
+  // This also checks that pid is a valid numeric string
   char *cmdline = get_cmdline(xatou(pid)); /* never NULL */
 
   // Create directory with the debug dump
@@ -109,11 +141,10 @@ int main(int argc, char** argv)
   snprintf(path, sizeof(path), DEBUG_DUMPS_DIR"/pyhook-%ld-%s",
 	   (long)time(NULL), pid);
   CDebugDump dd;
-
   try {
     dd.Create(path, getuid());
   } catch (CABRTException &e) {
-    error_msg_and_die("Error while creating debug dump: %s", e.what());
+    error_msg_and_die("error while creating crash dump %s: %s", path, e.what());
   }
 
   dd.SaveText(FILENAME_ANALYZER, "Python");
@@ -128,6 +159,8 @@ int main(int argc, char** argv)
   dd.SaveText("uid", uid);
 
   dd.Close();
+  log("saved python crash dump of pid %s to %s", pid, path);
+
   if (setting_MaxCrashReportsSize > 0)
   {
     trim_debug_dumps(setting_MaxCrashReportsSize, path);