From 546a6dfd0e929991e6136c07c489213916db8fa4 Mon Sep 17 00:00:00 2001
From: Andrei Vagin <avagin@gmail.com>
Date: Mon, 26 Jul 2021 05:38:17 +0300
Subject: [PATCH] configs: fix used after free cases

We have some code written with the assumption that argv is never
destroyed, but when we handle configs, we construct an argv-like array
for each config, parse it and release it.

I am still not sure that we need to release memory of per-config argv
arrays... The current scheme is going to be a source of used-after-free
bugs. When we will add the non-privileged mode, all these bugs will be
serious security issues.

Signed-off-by: Andrei Vagin <avagin@gmail.com>
---
 criu/action-scripts.c     |  8 ++++++--
 criu/cgroup.c             | 14 ++++++++++++--
 criu/config.c             |  1 -
 criu/external.c           | 22 +++++++++++++---------
 criu/filesystems.c        |  6 ++++--
 criu/include/cr_options.h |  4 +++-
 criu/namespaces.c         |  8 +++++++-
 7 files changed, 45 insertions(+), 18 deletions(-)

diff --git a/criu/action-scripts.c b/criu/action-scripts.c
index cc0118353..1ce6d9c10 100644
--- a/criu/action-scripts.c
+++ b/criu/action-scripts.c
@@ -147,9 +147,13 @@ int add_script(char *path)
 
 	script = xmalloc(sizeof(struct script));
 	if (script == NULL)
-		return 1;
+		return -1;
 
-	script->path = path;
+	script->path = xstrdup(path);
+	if (!script->path) {
+		xfree(script);
+		return -1;
+	}
 	list_add(&script->node, &scripts);
 
 	return 0;
diff --git a/criu/cgroup.c b/criu/cgroup.c
index 73461d6a0..ccac37fcc 100644
--- a/criu/cgroup.c
+++ b/criu/cgroup.c
@@ -1994,10 +1994,20 @@ int new_cg_root_add(char *controller, char *newroot)
 	if (!o)
 		return -1;
 
-	o->controller = controller;
-	o->newroot = newroot;
+	o->controller = xstrdup(controller);
+	if (!o->controller)
+		goto err_ctrl;
+	o->newroot = xstrdup(newroot);
+	if (!o->newroot)
+		goto err_newroot;
 	list_add(&o->node, &opts.new_cgroup_roots);
+
 	return 0;
+err_newroot:
+	xfree(o->controller);
+err_ctrl:
+	xfree(o);
+	return -1;
 }
 
 struct ns_desc cgroup_ns_desc = NS_DESC_ENTRY(CLONE_NEWCGROUP, "cgroup");
diff --git a/criu/config.c b/criu/config.c
index 4561c3d70..4e332d546 100644
--- a/criu/config.c
+++ b/criu/config.c
@@ -854,7 +854,6 @@ int parse_options(int argc, char **argv, bool *usage_error, bool *has_exec_cmd,
 			return 1;
 		case 'L':
 			SET_CHAR_OPTS(libdir, optarg);
-			opts.libdir = optarg;
 			break;
 		case 1059:
 			*has_exec_cmd = true;
diff --git a/criu/external.c b/criu/external.c
index 96e676849..bbbcd17cb 100644
--- a/criu/external.c
+++ b/criu/external.c
@@ -12,24 +12,28 @@ int add_external(char *key)
 {
 	struct external *ext;
 
+	if (strstartswith(key, "mnt[]"))
+		return ext_mount_parse_auto(key + 5);
+
 	ext = xmalloc(sizeof(*ext));
 	if (!ext)
 		return -1;
-	ext->id = key;
 
-	if (strstartswith(key, "macvlan") && macvlan_ext_add(ext) < 0) {
-		xfree(ext);
-		return -1;
-	}
+	ext->id = xstrdup(key);
+	if (!ext->id)
+		goto err_id;
 
-	if (strstartswith(key, "mnt[]")) {
-		xfree(ext);
-		return ext_mount_parse_auto(key + 5);
-	}
+	if (strstartswith(key, "macvlan") && macvlan_ext_add(ext) < 0)
+		goto err;
 
 	list_add(&ext->node, &opts.external);
 
 	return 0;
+err:
+	xfree(ext->id);
+err_id:
+	xfree(ext);
+	return -1;
 }
 
 bool external_lookup_id(char *id)
diff --git a/criu/filesystems.c b/criu/filesystems.c
index ee766a2f4..3e0ec2eb3 100644
--- a/criu/filesystems.c
+++ b/criu/filesystems.c
@@ -829,9 +829,11 @@ bool add_fsname_auto(const char *names)
 
 	if (css_contains(names, fsauto_all))
 		fsauto_names = fsauto_all;
-	else if (!old)
+	else if (!old) {
 		fsauto_names = xstrdup(names);
-	else {
+		if (!fsauto_names)
+			abort();
+	} else {
 		if (asprintf(&fsauto_names, "%s,%s", old, names) < 0)
 			fsauto_names = NULL;
 	}
diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
index 6836e8114..d46049cc9 100644
--- a/criu/include/cr_options.h
+++ b/criu/include/cr_options.h
@@ -18,7 +18,9 @@
 #define SET_CHAR_OPTS(__dest, __src)              \
 	do {                                      \
 		char *__src_dup = xstrdup(__src); \
-		free(opts.__dest);                \
+		if (!__src_dup)                   \
+			abort();                  \
+		xfree(opts.__dest);               \
 		opts.__dest = __src_dup;          \
 	} while (0)
 
diff --git a/criu/namespaces.c b/criu/namespaces.c
index b1a15dde8..92e0ed31a 100644
--- a/criu/namespaces.c
+++ b/criu/namespaces.c
@@ -147,7 +147,12 @@ int join_ns_add(const char *type, char *ns_file, char *extra_opts)
 	if (!jn)
 		return -1;
 
-	jn->ns_file = ns_file;
+	jn->ns_file = xstrdup(ns_file);
+	if (!jn->ns_file) {
+		xfree(jn);
+		return -1;
+	}
+
 	if (!strncmp(type, "net", 4)) {
 		jn->nd = &net_ns_desc;
 		join_ns_flags |= CLONE_NEWNET;
@@ -182,6 +187,7 @@ int join_ns_add(const char *type, char *ns_file, char *extra_opts)
 	pr_info("Added %s:%s join namespace\n", type, ns_file);
 	return 0;
 err:
+	xfree(jn->ns_file);
 	xfree(jn);
 	return -1;
 }