crash-manager: Santize cmd_line early 65/250065/2
authorKarol Lewandowski <k.lewandowsk@samsung.com>
Fri, 18 Dec 2020 11:24:18 +0000 (12:24 +0100)
committerKarol Lewandowski <k.lewandowsk@samsung.com>
Fri, 18 Dec 2020 12:52:18 +0000 (13:52 +0100)
There is no value in passing cmd_line that is potentially corrupted.

Previously, we wanted to share original cmdline value via dbus-notify
but this no longer seems correct as we have to replace cmd_line
completely when it contains invalid/non-printable characters.

This patch changes behaviour so that santized cmd_line is available
universally via cinfo->cmd_line.

Change-Id: I9a54781614d89c6abb89ef0c0755d8a23a26f55d

src/crash-manager/crash-manager.c

index e1ac790..2af5c1d 100644 (file)
@@ -289,16 +289,44 @@ static bool make_dump_dir(void)
        return true;
 }
 
+void get_proc_name(const char *cmd_line, char *buff, size_t buff_len)
+{
+       assert(buff != NULL);
+       char *space = strchr(cmd_line, ' ');
+       size_t name_len;
+
+       if (space == NULL)
+               name_len = strlen(cmd_line);
+       else
+               name_len = space - cmd_line;
+
+       if (name_len > buff_len - 1)
+               name_len = buff_len - 1;
+
+       // cmdline is valid only if it contains characters that can be used to
+       // to generate "valid" crash report name. Characters > 127 are rejected.
+       for (size_t z = 0; z < name_len; z++) {
+               if (!isgraph(cmd_line[z])) {
+                       strncpy(buff, "corrupted_cmdline", buff_len);
+                       return;
+               }
+       }
+
+       strncpy(buff, cmd_line, name_len);
+       buff[name_len] = '\0';
+}
+
 static bool get_cmd_info(struct crash_info *cinfo)
 {
        assert(cinfo);
 
        char buf[PATH_MAX];
+       char proc_name[PATH_MAX];
 
        if (!read_proc_file(cinfo->pid_info, "cmdline", buf, sizeof(buf), NULL))
                goto err;
-
-       cinfo->cmd_line = strdup(buf);
+       get_proc_name(buf, proc_name, sizeof(proc_name));
+       cinfo->cmd_line = strdup(proc_name);
        if (!cinfo->cmd_line)
                return false;
 
@@ -375,33 +403,6 @@ close_fd:
        return -1;
 }
 
-void get_proc_name(const char *cmd_line, char *buff, size_t buff_len)
-{
-       assert(buff != NULL);
-       char *space = strchr(cmd_line, ' ');
-       size_t name_len;
-
-       if (space == NULL)
-               name_len = strlen(cmd_line);
-       else
-               name_len = space - cmd_line;
-
-       if (name_len > buff_len - 1)
-               name_len = buff_len - 1;
-
-       // cmdline is valid only if it contains characters that can be used to
-       // to generate "valid" crash report name. Characters > 127 are rejected.
-       for (size_t z = 0; z < name_len; z++) {
-               if (!isgraph(cmd_line[z])) {
-                       strncpy(buff, "corrupted_cmdline", buff_len);
-                       return;
-               }
-       }
-
-       strncpy(buff, cmd_line, name_len);
-       buff[name_len] = '\0';
-}
-
 static void set_crash_info_defaults(struct crash_info *cinfo)
 {
        if (cinfo->livedump) {
@@ -505,14 +506,12 @@ bool set_crash_info(struct crash_info *cinfo)
                return false;
        }
 
-       char proc_name[PATH_MAX];
-       get_proc_name(cinfo->cmd_line, proc_name, sizeof(proc_name));
        const char *suffix = config.report_type >= REP_TYPE_FULL ? (config.allow_zip ? ".zip" : "") : ".info";
        bool is_app = set_appinfo(cinfo->cmd_line, &cinfo->appid, &cinfo->pkgid);
        if (!cinfo->appid || !cinfo->pkgid)
                goto out_oom;
 
-       if (-1 == asprintf(&cinfo->name,        "%s_%d_%s",        is_app ? cinfo->pkgid : basename(proc_name), cinfo->pid_info, date)
+       if (-1 == asprintf(&cinfo->name,        "%s_%d_%s",        is_app ? cinfo->pkgid : basename(cinfo->cmd_line), cinfo->pid_info, date)
         || -1 == asprintf(&cinfo->pfx,         "%s/%s",           cinfo->temp_dir, cinfo->name)
         || -1 == asprintf(&cinfo->info_path,   "%s/%s.info",      cinfo->pfx, cinfo->name)
         || -1 == asprintf(&cinfo->info_json,   "%s/%s.info.json", cinfo->pfx, cinfo->name)
@@ -691,9 +690,6 @@ static bool execute_minicoredump(struct crash_info *cinfo, int *exit_code)
 
        const char *without_core_str = config.dump_core ? NULL : "-s";
 
-       char proc_name[PATH_MAX];
-       get_proc_name(cinfo->cmd_line, proc_name, sizeof(proc_name));
-
        /* Execute minicoredumper */
        char *args[] = {
                        MINICOREDUMPER_BIN_PATH,   // minicoredumper filename path
@@ -703,7 +699,7 @@ static bool execute_minicoredump(struct crash_info *cinfo, int *exit_code)
                        sig_str,                   // %s - number of signal
                        time_str,                  // %t - time of dump
                        "localhost",               // %h - hostname
-                       basename(proc_name), // %e - exe name (need for result filename)
+                       basename(cinfo->cmd_line), // %e - exe name (need for result filename)
                        MINICOREDUMPER_CONFIG_PATH, // config file
                        "-d",
                        cinfo->pfx,            // temp dir