Skip to content

Commit

Permalink
status: validate container id
Browse files Browse the repository at this point in the history
check that the container name does not contain any slash so that
operations in status.c won't risk to access directories/files outside
of the run directory.

Beside, it is not a security issue because if a directory exists, crun
refuses to use it; and if it does not exist then it is created and
immediately deleted because the container creation fails.

Play safe and do not allow any slash in the container name.

Closes: #1646

Signed-off-by: Giuseppe Scrivano <[email protected]>
  • Loading branch information
giuseppe committed Jan 28, 2025
1 parent 4d1ecdd commit 564dd37
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 0 deletions.
22 changes: 22 additions & 0 deletions src/libcrun/status.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ struct pid_stat
unsigned long long starttime;
};

/* If ID is not NULL, then ennsure that it does not contain any slash. */
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);

return 0;
}

static int
get_run_directory (char **out, const char *state_root, libcrun_error_t *err)
{
Expand Down Expand Up @@ -82,6 +92,10 @@ libcrun_get_state_directory (char **out, const char *state_root, const char *id,
cleanup_free char *path = NULL;
cleanup_free char *root = NULL;

ret = validate_id (id, err);
if (UNLIKELY (ret < 0))
return ret;

ret = get_run_directory (&root, state_root, err);
if (UNLIKELY (ret < 0))
return ret;
Expand All @@ -102,6 +116,10 @@ get_state_directory_status_file (char **out, const char *state_root, const char
char *path = NULL;
int ret;

ret = validate_id (id, err);
if (UNLIKELY (ret < 0))
return ret;

ret = get_run_directory (&root, state_root, err);
if (UNLIKELY (ret < 0))
return ret;
Expand Down Expand Up @@ -551,6 +569,10 @@ libcrun_container_delete_status (const char *state_root, const char *id, libcrun
cleanup_close int dfd = -1;
cleanup_free char *dir = NULL;

ret = validate_id (id, err);
if (UNLIKELY (ret < 0))
return ret;

ret = get_run_directory (&dir, state_root, err);
if (UNLIKELY (ret < 0))
return ret;
Expand Down
17 changes: 17 additions & 0 deletions tests/test_start.py
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,22 @@ def test_run_keep():

return 0

def test_invalid_id():
conf = base_config()
conf['process']['args'] = ['./init', 'echo', 'hello']
conf['process']['cwd'] = "/sbin"
add_all_namespaces(conf)
try:
out, _ = run_and_get_output(conf, id_container="this/is/invalid")
return -1
except Exception as e:
err = e.output.decode()
if "invalid character `/` in the ID" in err:
return 0
sys.stderr.write("Got error: %s\n" % err)
return -1
return 0

all_tests = {
"start" : test_start,
"start-override-config" : test_start_override_config,
Expand All @@ -562,6 +578,7 @@ def test_run_keep():
"unknown-sysctl": test_unknown_sysctl,
"ioprio": test_ioprio,
"run-keep": test_run_keep,
"invalid-id": test_invalid_id,
}

if __name__ == "__main__":
Expand Down

0 comments on commit 564dd37

Please sign in to comment.