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>
This commit is contained in:
Andrei Vagin 2021-07-26 05:38:17 +03:00
parent 399a53a43f
commit 546a6dfd0e
7 changed files with 45 additions and 18 deletions

View file

@ -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;

View file

@ -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");

View file

@ -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;

View file

@ -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)

View file

@ -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;
}

View file

@ -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)

View file

@ -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;
}