From e08cdc8562f55b9ac228a21f3f7605a18c522b81 Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Fri, 6 Feb 2026 11:01:29 +0000 Subject: [PATCH 1/3] hotplug-dispatch: fix filter disallowing setting PATH Due to a bug in hotplug-dispatch, the PATH env variable wasn't filtered, allowing authrorized callers the execution of commands via PATH environment variable filter bypass. Replace the call to strcmp with strncmp and limit the comparision to 5 characters to account for each character in "PATH=". Fixes: TOB-OWRT-4 Fixes: 08938fe ("procd: add hotplug-call dispatcher") Reported-by: Trail of Bits (@trailofbits) Signed-off-by: Daniel Golle --- hotplug-dispatch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hotplug-dispatch.c b/hotplug-dispatch.c index f048fcb..a67ef2d 100644 --- a/hotplug-dispatch.c +++ b/hotplug-dispatch.c @@ -232,7 +232,7 @@ static int hotplug_call(struct ubus_context *ctx, struct ubus_object *obj, if (!strncmp(enve, "LD_", 3)) continue; - if (!strcmp(enve, "PATH")) + if (!strncmp(enve, "PATH=", 5)) continue; if (strlen(enve) < 3) From 014f94cd857df0a66e57d4c205fd28edacf11a4e Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Fri, 6 Feb 2026 11:10:28 +0000 Subject: [PATCH 2/3] procd: jail/cgroups: fix OOB write in cgroups_apply() Check if any cgroups have been selected and string subtree_control has a length greater than 0 before reducing its length by 1, preventing to write outside of the bounds of the array in case no cgroups are selected. Fixes: ID: TOB-OWRT-6 Fixes: 16159bb ("jail: parse OCI cgroups resources") Reported-by: Trail of Bits Signed-off-by: Daniel Golle --- jail/cgroups.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/jail/cgroups.c b/jail/cgroups.c index 2d3dce4..1eb229b 100644 --- a/jail/cgroups.c +++ b/jail/cgroups.c @@ -164,9 +164,12 @@ void cgroups_apply(pid_t pid) if (rdma) strcat(subtree_control, "+rdma "); - /* remove trailing space */ - ent = strchr(subtree_control, '\0') - 1; - *ent = '\0'; + /* remove trailing space (length is > 0) */ + ent = strchr(subtree_control, '\0'); + if (ent > subtree_control) { + ent -= 1; + *ent = '\0'; + } ent = malloc(maxlen); if (!ent) From 7e5b324cc8fee8a315debd5f8e1e5ea9badc903c Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Fri, 6 Feb 2026 11:31:42 +0000 Subject: [PATCH 3/3] instance: check length of names when creating cgroups A stack buffer overflow may occur during path construction in instance_add_cgroup if the snprintf calls before the strcat call fill the 256-byte stack buffer. Check the length at all stages when creating and appending the string in the buffer and return an error in case it gets to long. Fixes: 83053b6 ("instance: add instances into unified cgroup hierarchy") Fixes: TOB-OWRT-8 Reported-by: Trail of Bits Signed-off-by: Daniel Golle --- service/instance.c | 46 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/service/instance.c b/service/instance.c index 2b8bba0..c559cda 100644 --- a/service/instance.c +++ b/service/instance.c @@ -559,30 +559,48 @@ instance_run(struct service_instance *in, int _stdout, int _stderr) exit(127); } -static void +static int instance_add_cgroup(const char *service, const char *instance) { - struct stat sb; + const char *cgroup_procs = "/cgroup.procs"; char cgnamebuf[256]; - int fd; + struct stat sb; + int fd, ret; if (stat("/sys/fs/cgroup/cgroup.subtree_control", &sb)) - return; + return -ENOENT; mkdir(CGROUP_BASEDIR, 0700); - snprintf(cgnamebuf, sizeof(cgnamebuf), "%s/%s", CGROUP_BASEDIR, service); - mkdir(cgnamebuf, 0700); - snprintf(cgnamebuf, sizeof(cgnamebuf), "%s/%s/%s", CGROUP_BASEDIR, service, instance); - mkdir(cgnamebuf, 0700); - strcat(cgnamebuf, "/cgroup.procs"); + ret = snprintf(cgnamebuf, sizeof(cgnamebuf), "%s/%s", CGROUP_BASEDIR, + service); + if (ret >= sizeof(cgnamebuf)) + return -ENAMETOOLONG; + + if (mkdir(cgnamebuf, 0700)) + return -EPERM; + + ret = snprintf(cgnamebuf, sizeof(cgnamebuf), "%s/%s/%s", CGROUP_BASEDIR, + service, instance); + if (ret >= sizeof(cgnamebuf)) + return -ENAMETOOLONG; + + if (mkdir(cgnamebuf, 0700)) + return -EPERM; + + if (strlen(cgnamebuf) + strlen(cgroup_procs) >= sizeof(cgnamebuf)) + return -ENAMETOOLONG; + + strcat(cgnamebuf, cgroup_procs); fd = open(cgnamebuf, O_WRONLY); if (fd == -1) - return; + return -EIO; dprintf(fd, "%d", getpid()); close(fd); + + return 0; } static void @@ -616,7 +634,7 @@ instance_free_stdio(struct service_instance *in) void instance_start(struct service_instance *in) { - int pid; + int pid, ret; int opipe[2] = { -1, -1 }; int epipe[2] = { -1, -1 }; @@ -665,7 +683,11 @@ instance_start(struct service_instance *in) uloop_done(); closefd(opipe[0]); closefd(epipe[0]); - instance_add_cgroup(in->srv->name, in->name); + ret = instance_add_cgroup(in->srv->name, in->name); + if (ret) + ULOG_WARN("failed adding instance cgroup for %s: %s\n", + in->srv->name, strerror(ret)); + instance_run(in, opipe[1], epipe[1]); return; }