From a79f422a51adabd0eb8c8727a7331f4c14f9deba Mon Sep 17 00:00:00 2001
From: Karol Lewandowski <k.lewandowsk@samsung.com>
Date: Fri, 18 Dec 2020 12:24:18 +0100
Subject: [PATCH] crash-manager: Santize cmd_line early

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 | 68 ++++++++++++++++++---------------------
 1 file changed, 32 insertions(+), 36 deletions(-)

diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c
index e1ac790..2af5c1d 100644
--- a/src/crash-manager/crash-manager.c
+++ b/src/crash-manager/crash-manager.c
@@ -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
-- 
2.7.4