diff --git a/src/libcrun/cgroup.c b/src/libcrun/cgroup.c index 63f43d7cf2..7c6a9ddd0e 100644 --- a/src/libcrun/cgroup.c +++ b/src/libcrun/cgroup.c @@ -390,6 +390,9 @@ libcrun_cgroup_enter_finalize (struct libcrun_cgroup_args *args, struct libcrun_ if (delegate_cgroup == NULL) return 0; + if (path_has_dot_dot_component (delegate_cgroup)) + return crun_make_error (err, 0, "invalid `..` component in delegate-cgroup `%s`", delegate_cgroup); + cgroup_mode = libcrun_get_cgroup_mode (err); if (UNLIKELY (cgroup_mode < 0)) return cgroup_mode; diff --git a/src/libcrun/status.c b/src/libcrun/status.c index 1eb96c3502..e32a5baf8b 100644 --- a/src/libcrun/status.c +++ b/src/libcrun/status.c @@ -45,12 +45,31 @@ struct pid_stat unsigned long long starttime; }; -/* If ID is not NULL, then ennsure that it does not contain any slash. */ +static inline bool +is_valid_id_char (char c, bool first) +{ + if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9')) + return true; + if (first) + return false; + return c == '_' || c == '.' || c == '-'; +} + +/* Ensure the ID is not empty and matches [a-zA-Z0-9][a-zA-Z0-9_.-]*. */ static int validate_id (const char *id, libcrun_error_t *err) { - if (id && strchr (id, '/') != NULL) - return crun_make_error (err, 0, "invalid character `/` in the ID `%s`", id); + if (id == NULL) + return 0; + + if (id[0] == '\0') + return crun_make_error (err, 0, "empty container ID"); + + for (size_t i = 0; id[i]; i++) + { + if (! is_valid_id_char (id[i], i == 0)) + return crun_make_error (err, 0, "invalid character `%c` in the container ID `%s`", id[i], id); + } return 0; } diff --git a/src/libcrun/utils.h b/src/libcrun/utils.h index 1aa378d952..268dff58e9 100644 --- a/src/libcrun/utils.h +++ b/src/libcrun/utils.h @@ -404,6 +404,34 @@ is_empty_string (const char *s) return s == NULL || s[0] == '\0'; } +static inline bool +path_has_dot_dot_component (const char *path) +{ + if (path == NULL) + return false; + cleanup_free char *npath = xstrdup (path); + char *cur, *it; + + cur = npath; + it = strchr (cur, '/'); + while (cur) + { + if (it) + *it = '\0'; + + if (strcmp (cur, "..") == 0) + return true; + + if (it == NULL) + break; + + cur = consume_slashes (it + 1); + *it = '/'; + it = strchr (cur, '/'); + } + return false; +} + static inline int waitpid_ignore_stopped (pid_t pid, int *status, int options) { diff --git a/tests/test_cgroup_setup.py b/tests/test_cgroup_setup.py index 42e24e4bac..e556629332 100755 --- a/tests/test_cgroup_setup.py +++ b/tests/test_cgroup_setup.py @@ -1298,6 +1298,39 @@ def test_annotation_delegate_cgroup(): run_crun_command(["delete", "-f", cid]) +def test_annotation_delegate_cgroup_dotdot(): + """Test that run.oci.delegate-cgroup rejects '..' components.""" + if not is_cgroup_v2_unified(): + return (77, "requires cgroup v2") + if not running_on_systemd(): + return (77, "requires systemd") + if get_cgroup_manager() != 'systemd': + return (77, "requires systemd cgroup manager") + + conf = base_config() + add_all_namespaces(conf, cgroupns=True) + conf['process']['args'] = ['/init', 'echo', 'hello'] + + if 'annotations' not in conf: + conf['annotations'] = {} + conf['annotations']['run.oci.systemd.subgroup'] = 'mysubgroup' + + for invalid in ["../../../victim", "../escape", "a/../b"]: + conf['annotations']['run.oci.delegate-cgroup'] = invalid + try: + out, _ = run_and_get_output(conf, hide_stderr=True) + return -1 + except Exception as e: + if hasattr(e, 'output') and e.output: + err = e.output.decode() + if "delegate-cgroup" in err: + continue + logger.info("got error for delegate-cgroup '%s': %s", invalid, err) + else: + logger.info("got exception without output for delegate-cgroup '%s': %s", invalid, str(e)) + return -1 + return 0 + def test_annotation_systemd_force_cgroup_v1(): """Test run.oci.systemd.force_cgroup_v1 annotation.""" if not is_cgroup_v2_unified(): @@ -1544,6 +1577,7 @@ def test_cgroup_subcgroup_cleanup(): "cgroup-create-without-resources": test_cgroup_create_without_resources, "annotation-systemd-subgroup": test_annotation_systemd_subgroup, "annotation-delegate-cgroup": test_annotation_delegate_cgroup, + "annotation-delegate-cgroup-dotdot": test_annotation_delegate_cgroup_dotdot, "annotation-systemd-force-cgroup-v1": test_annotation_systemd_force_cgroup_v1, "cgroup-v2-mount-options": test_cgroup_v2_mount_options, "cgroup-subcgroup-cleanup": test_cgroup_subcgroup_cleanup, diff --git a/tests/test_start.py b/tests/test_start.py index ec0c025db1..23c08267b4 100755 --- a/tests/test_start.py +++ b/tests/test_start.py @@ -523,7 +523,7 @@ def test_invalid_id(): except Exception as e: if hasattr(e, 'output') and e.output: err = e.output.decode() - if "invalid character `/` in the ID" in err: + if "invalid character" in err and "container ID" in err: return 0 logger.info("got error: %s", err) else: @@ -531,6 +531,26 @@ def test_invalid_id(): return -1 return 0 +def test_invalid_id_chars(): + conf = base_config() + conf['process']['args'] = ['./init', 'echo', 'hello'] + conf['process']['cwd'] = "/sbin" + add_all_namespaces(conf) + for invalid_id in [".", "..", ".foo", "_foo", "foo bar", "foo/bar"]: + try: + out, _ = run_and_get_output(conf, id_container=invalid_id) + return -1 + except Exception as e: + if hasattr(e, 'output') and e.output: + err = e.output.decode() + if "invalid" in err.lower(): + continue + logger.info("got error for id '%s': %s", invalid_id, err) + else: + logger.info("got exception without output for id '%s': %s", invalid_id, str(e)) + return -1 + return 0 + def test_home_unknown_id(): if is_rootless(): return (77, "requires root privileges") @@ -610,6 +630,7 @@ def test_systemd_cgroups_path_def_slice(): "ioprio": test_ioprio, "run-keep": test_run_keep, "invalid-id": test_invalid_id, + "invalid-id-chars": test_invalid_id_chars, "home-unknown-id": test_home_unknown_id, "help": test_start_help, "systemd-cgroups-path-def-slice": test_systemd_cgroups_path_def_slice,