mirror of
https://github.com/openwrt/openwrt.git
synced 2026-10-13 19:48:16 +04:00
rpcd: cherry pick patches from main
This fixes multiple bugs, some might be security relevant. Link: https://github.com/openwrt/openwrt/pull/24330 Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
This commit is contained in:
1 parent
6cfbe534c2
commit
b421c63071
12 files changed
+1122
-1
No files matched your search
@@ -8,7 +8,7 @@
|
||||
include $(TOPDIR)/rules.mk
|
||||
|
||||
PKG_NAME:=rpcd
|
||||
PKG_RELEASE:=1
|
||||
PKG_RELEASE:=2
|
||||
|
||||
PKG_SOURCE_PROTO:=git
|
||||
PKG_SOURCE_URL=$(PROJECT_GIT)/project/rpcd.git
|
||||
|
||||
+238
@@ -0,0 +1,238 @@
|
||||
From e19b63ae4a4ffa6ef23dc1bb059d0fdf602fa078 Mon Sep 17 00:00:00 2001
|
||||
From: Meng <x.meng@genexis.eu>
|
||||
Date: Mon, 16 Mar 2026 09:58:36 +0100
|
||||
Subject: exec: defer async reply teardown to avoid use-after-free
|
||||
|
||||
Async exec replies currently tear down their context directly from
|
||||
ustream callbacks. When the callback chain keeps running after the
|
||||
reply path frees the context, rpcd can hit a use-after-free and crash
|
||||
with SIGSEGV.
|
||||
|
||||
Defer reply completion through a 0 ms uloop timeout and guard against
|
||||
duplicate scheduling so cleanup runs after the callbacks unwind in both
|
||||
exec paths.
|
||||
|
||||
Signed-off-by: Meng <x.meng@genexis.eu>
|
||||
(cherry picked from commit e655a0d69492a91d9dada74f0832e028e6b5367b)
|
||||
---
|
||||
exec.c | 36 +++++++++++++++++++++++++++++++-----
|
||||
file.c | 39 ++++++++++++++++++++++++++++++++++-----
|
||||
include/rpcd/exec.h | 3 +++
|
||||
3 files changed, 68 insertions(+), 10 deletions(-)
|
||||
|
||||
--- a/exec.c
|
||||
+++ b/exec.c
|
||||
@@ -122,6 +122,9 @@ rpc_exec_reply(struct rpc_exec_context *
|
||||
{
|
||||
uloop_timeout_cancel(&c->timeout);
|
||||
uloop_process_delete(&c->process);
|
||||
+ uloop_timeout_cancel(&c->deferred_reply);
|
||||
+
|
||||
+ c->deferred_reply_pending = false;
|
||||
|
||||
if (rv == UBUS_STATUS_OK)
|
||||
{
|
||||
@@ -156,13 +159,34 @@ rpc_exec_reply(struct rpc_exec_context *
|
||||
}
|
||||
|
||||
static void
|
||||
+rpc_exec_deferred_reply_cb(struct uloop_timeout *t)
|
||||
+{
|
||||
+ struct rpc_exec_context *c =
|
||||
+ container_of(t, struct rpc_exec_context, deferred_reply);
|
||||
+
|
||||
+ c->deferred_reply_pending = false;
|
||||
+ rpc_exec_reply(c, c->deferred_status);
|
||||
+}
|
||||
+
|
||||
+static void
|
||||
+rpc_exec_schedule_reply(struct rpc_exec_context *c, int rv)
|
||||
+{
|
||||
+ if (c->deferred_reply_pending)
|
||||
+ return;
|
||||
+
|
||||
+ c->deferred_status = rv;
|
||||
+ c->deferred_reply_pending = true;
|
||||
+ uloop_timeout_set(&c->deferred_reply, 0);
|
||||
+}
|
||||
+
|
||||
+static void
|
||||
rpc_exec_timeout_cb(struct uloop_timeout *t)
|
||||
{
|
||||
struct rpc_exec_context *c =
|
||||
container_of(t, struct rpc_exec_context, timeout);
|
||||
|
||||
kill(c->process.pid, SIGKILL);
|
||||
- rpc_exec_reply(c, UBUS_STATUS_TIMEOUT);
|
||||
+ rpc_exec_schedule_reply(c, UBUS_STATUS_TIMEOUT);
|
||||
}
|
||||
|
||||
static void
|
||||
@@ -222,7 +246,7 @@ rpc_exec_opipe_read_cb(struct ustream *s
|
||||
}
|
||||
else if (ustream_read_buf_full(s))
|
||||
{
|
||||
- rpc_exec_reply(c, UBUS_STATUS_NOT_SUPPORTED);
|
||||
+ rpc_exec_schedule_reply(c, UBUS_STATUS_NOT_SUPPORTED);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -252,7 +276,7 @@ rpc_exec_epipe_read_cb(struct ustream *s
|
||||
}
|
||||
else if (ustream_read_buf_full(s))
|
||||
{
|
||||
- rpc_exec_reply(c, UBUS_STATUS_NOT_SUPPORTED);
|
||||
+ rpc_exec_schedule_reply(c, UBUS_STATUS_NOT_SUPPORTED);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -263,7 +287,7 @@ rpc_exec_opipe_state_cb(struct ustream *
|
||||
container_of(s, struct rpc_exec_context, opipe.stream);
|
||||
|
||||
if (c->opipe.stream.eof && c->epipe.stream.eof)
|
||||
- rpc_exec_reply(c, UBUS_STATUS_OK);
|
||||
+ rpc_exec_schedule_reply(c, UBUS_STATUS_OK);
|
||||
}
|
||||
|
||||
static void
|
||||
@@ -273,7 +297,7 @@ rpc_exec_epipe_state_cb(struct ustream *
|
||||
container_of(s, struct rpc_exec_context, epipe.stream);
|
||||
|
||||
if (c->opipe.stream.eof && c->epipe.stream.eof)
|
||||
- rpc_exec_reply(c, UBUS_STATUS_OK);
|
||||
+ rpc_exec_schedule_reply(c, UBUS_STATUS_OK);
|
||||
}
|
||||
|
||||
int
|
||||
@@ -349,6 +373,8 @@ rpc_exec(const char **args, rpc_exec_wri
|
||||
c->process.cb = rpc_exec_process_cb;
|
||||
uloop_process_add(&c->process);
|
||||
|
||||
+ c->deferred_reply.cb = rpc_exec_deferred_reply_cb;
|
||||
+
|
||||
c->timeout.cb = rpc_exec_timeout_cb;
|
||||
uloop_timeout_set(&c->timeout, rpc_exec_timeout);
|
||||
|
||||
--- a/file.c
|
||||
+++ b/file.c
|
||||
@@ -68,7 +68,10 @@ struct rpc_file_exec_context {
|
||||
struct uloop_process process;
|
||||
struct ustream_fd opipe;
|
||||
struct ustream_fd epipe;
|
||||
+ struct uloop_timeout deferred_reply;
|
||||
int stat;
|
||||
+ int deferred_status;
|
||||
+ bool deferred_reply_pending;
|
||||
};
|
||||
|
||||
|
||||
@@ -696,6 +699,9 @@ rpc_file_exec_reply(struct rpc_file_exec
|
||||
{
|
||||
uloop_timeout_cancel(&c->timeout);
|
||||
uloop_process_delete(&c->process);
|
||||
+ uloop_timeout_cancel(&c->deferred_reply);
|
||||
+
|
||||
+ c->deferred_reply_pending = false;
|
||||
|
||||
if (rv == UBUS_STATUS_OK)
|
||||
{
|
||||
@@ -722,13 +728,34 @@ rpc_file_exec_reply(struct rpc_file_exec
|
||||
}
|
||||
|
||||
static void
|
||||
+rpc_file_exec_deferred_reply_cb(struct uloop_timeout *t)
|
||||
+{
|
||||
+ struct rpc_file_exec_context *c =
|
||||
+ container_of(t, struct rpc_file_exec_context, deferred_reply);
|
||||
+
|
||||
+ c->deferred_reply_pending = false;
|
||||
+ rpc_file_exec_reply(c, c->deferred_status);
|
||||
+}
|
||||
+
|
||||
+static void
|
||||
+rpc_file_exec_schedule_reply(struct rpc_file_exec_context *c, int rv)
|
||||
+{
|
||||
+ if (c->deferred_reply_pending)
|
||||
+ return;
|
||||
+
|
||||
+ c->deferred_status = rv;
|
||||
+ c->deferred_reply_pending = true;
|
||||
+ uloop_timeout_set(&c->deferred_reply, 0);
|
||||
+}
|
||||
+
|
||||
+static void
|
||||
rpc_file_exec_timeout_cb(struct uloop_timeout *t)
|
||||
{
|
||||
struct rpc_file_exec_context *c =
|
||||
container_of(t, struct rpc_file_exec_context, timeout);
|
||||
|
||||
kill(c->process.pid, SIGKILL);
|
||||
- rpc_file_exec_reply(c, UBUS_STATUS_TIMEOUT);
|
||||
+ rpc_file_exec_schedule_reply(c, UBUS_STATUS_TIMEOUT);
|
||||
}
|
||||
|
||||
static void
|
||||
@@ -750,7 +777,7 @@ rpc_file_exec_opipe_read_cb(struct ustre
|
||||
container_of(s, struct rpc_file_exec_context, opipe.stream);
|
||||
|
||||
if (ustream_read_buf_full(s))
|
||||
- rpc_file_exec_reply(c, UBUS_STATUS_NOT_SUPPORTED);
|
||||
+ rpc_file_exec_schedule_reply(c, UBUS_STATUS_NOT_SUPPORTED);
|
||||
}
|
||||
|
||||
static void
|
||||
@@ -760,7 +787,7 @@ rpc_file_exec_epipe_read_cb(struct ustre
|
||||
container_of(s, struct rpc_file_exec_context, epipe.stream);
|
||||
|
||||
if (ustream_read_buf_full(s))
|
||||
- rpc_file_exec_reply(c, UBUS_STATUS_NOT_SUPPORTED);
|
||||
+ rpc_file_exec_schedule_reply(c, UBUS_STATUS_NOT_SUPPORTED);
|
||||
}
|
||||
|
||||
static void
|
||||
@@ -770,7 +797,7 @@ rpc_file_exec_opipe_state_cb(struct ustr
|
||||
container_of(s, struct rpc_file_exec_context, opipe.stream);
|
||||
|
||||
if (c->opipe.stream.eof && c->epipe.stream.eof)
|
||||
- rpc_file_exec_reply(c, UBUS_STATUS_OK);
|
||||
+ rpc_file_exec_schedule_reply(c, UBUS_STATUS_OK);
|
||||
}
|
||||
|
||||
static void
|
||||
@@ -780,7 +807,7 @@ rpc_file_exec_epipe_state_cb(struct ustr
|
||||
container_of(s, struct rpc_file_exec_context, epipe.stream);
|
||||
|
||||
if (c->opipe.stream.eof && c->epipe.stream.eof)
|
||||
- rpc_file_exec_reply(c, UBUS_STATUS_OK);
|
||||
+ rpc_file_exec_schedule_reply(c, UBUS_STATUS_OK);
|
||||
}
|
||||
|
||||
static void
|
||||
@@ -942,6 +969,8 @@ rpc_file_exec_run(const char *cmd, const
|
||||
c->process.cb = rpc_file_exec_process_cb;
|
||||
uloop_process_add(&c->process);
|
||||
|
||||
+ c->deferred_reply.cb = rpc_file_exec_deferred_reply_cb;
|
||||
+
|
||||
c->timeout.cb = rpc_file_exec_timeout_cb;
|
||||
uloop_timeout_set(&c->timeout, *ops->exec_timeout);
|
||||
|
||||
--- a/include/rpcd/exec.h
|
||||
+++ b/include/rpcd/exec.h
|
||||
@@ -61,6 +61,7 @@ struct rpc_exec_context {
|
||||
struct ubus_request_data request;
|
||||
struct uloop_timeout timeout;
|
||||
struct uloop_process process;
|
||||
+ struct uloop_timeout deferred_reply;
|
||||
struct ustream_fd ipipe;
|
||||
struct ustream_fd opipe;
|
||||
struct ustream_fd epipe;
|
||||
@@ -69,6 +70,8 @@ struct rpc_exec_context {
|
||||
int errlen;
|
||||
char *err;
|
||||
int stat;
|
||||
+ int deferred_status;
|
||||
+ bool deferred_reply_pending;
|
||||
void *priv;
|
||||
bool blob_array;
|
||||
void *blob_cookie;
|
||||
+88
@@ -0,0 +1,88 @@
|
||||
From e09c041c8fb027dde39ef617e24285eb1c654503 Mon Sep 17 00:00:00 2001
|
||||
From: Breeze <chanlikessummer@gmail.com>
|
||||
Date: Thu, 16 Apr 2026 23:56:19 +0800
|
||||
Subject: session: clamp uloop timeout to avoid int overflow
|
||||
|
||||
rpc_touch_session() computes `ses->timeout * 1000` as int*int, which
|
||||
overflows INT_MAX once ses->timeout exceeds 2147483 seconds (~24.85
|
||||
days). The wrapped-around negative value passed to uloop_timeout_set()
|
||||
causes libubox to fire the session timeout callback on the very next
|
||||
uloop iteration, destroying the just-created session before the caller
|
||||
can use it.
|
||||
|
||||
In practice this makes `ubus call session login` with e.g.
|
||||
`timeout:2592000` (30 days) return a valid-looking session id whose
|
||||
reported `expires` is a large negative number, and any subsequent
|
||||
session.set / session.get call on that SID returns "Not found". LuCI's
|
||||
ucode dispatcher passes the value of `luci.sauth.sessiontime` straight
|
||||
through as the login timeout, so users who follow common advice to bump
|
||||
sessiontime to 30 days get silently locked out of LuCI (uhttpd logs
|
||||
`accepted login`, response is 403 with no Set-Cookie) while SSH keeps
|
||||
working with the same credentials. At least one affected user resorted
|
||||
to factory-resetting multiple APs before recovering, see
|
||||
https://forum.openwrt.org/t/241892 .
|
||||
|
||||
The same overflow lurks in rpc_session_from_blob() when thawing a
|
||||
persisted session, where `blobmsg_get_u64(EXPIRES) * 1000` is passed to
|
||||
uloop_timeout_set()'s int `msecs` parameter.
|
||||
|
||||
Introduce a small helper that converts a seconds value to milliseconds
|
||||
with clamping to INT_MAX (and negative-input guard), and use it at both
|
||||
call sites. The cap still allows uloop timeouts of ~24.85 days, which
|
||||
is longer than any realistic administrative session.
|
||||
|
||||
Signed-off-by: Breeze <chanlikessummer@gmail.com>
|
||||
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
||||
(cherry picked from commit d005c885dbe439745abe7f64710cb93f5da569ce)
|
||||
---
|
||||
session.c | 28 ++++++++++++++++++++++++++--
|
||||
1 file changed, 26 insertions(+), 2 deletions(-)
|
||||
|
||||
--- a/session.c
|
||||
+++ b/session.c
|
||||
@@ -248,11 +248,34 @@ rpc_session_dump(struct rpc_session *ses
|
||||
ubus_send_reply(ctx, req, buf.head);
|
||||
}
|
||||
|
||||
+/*
|
||||
+ * Convert a session timeout in seconds to a millisecond value suitable
|
||||
+ * for uloop_timeout_set(), clamping to INT_MAX to avoid overflowing the
|
||||
+ * int argument. Without this, any timeout exceeding ~2147483 seconds
|
||||
+ * (~24.85 days) would wrap around to a negative value and cause libubox
|
||||
+ * to fire the timeout callback on the next uloop iteration, destroying
|
||||
+ * the session immediately after creation.
|
||||
+ */
|
||||
+static int
|
||||
+rpc_session_timeout_ms(int64_t seconds)
|
||||
+{
|
||||
+ int64_t msecs;
|
||||
+
|
||||
+ if (seconds < 0)
|
||||
+ seconds = 0;
|
||||
+
|
||||
+ msecs = seconds * 1000;
|
||||
+ if (msecs > INT_MAX)
|
||||
+ msecs = INT_MAX;
|
||||
+
|
||||
+ return (int)msecs;
|
||||
+}
|
||||
+
|
||||
static void
|
||||
rpc_touch_session(struct rpc_session *ses)
|
||||
{
|
||||
if (ses->timeout > 0)
|
||||
- uloop_timeout_set(&ses->t, ses->timeout * 1000);
|
||||
+ uloop_timeout_set(&ses->t, rpc_session_timeout_ms(ses->timeout));
|
||||
}
|
||||
|
||||
static void
|
||||
@@ -1315,7 +1338,8 @@ rpc_session_from_blob(struct uci_context
|
||||
|
||||
avl_insert(&sessions, &ses->avl);
|
||||
|
||||
- uloop_timeout_set(&ses->t, blobmsg_get_u64(tb[RPC_DUMP_EXPIRES]) * 1000);
|
||||
+ uloop_timeout_set(&ses->t,
|
||||
+ rpc_session_timeout_ms(blobmsg_get_u64(tb[RPC_DUMP_EXPIRES])));
|
||||
|
||||
return true;
|
||||
}
|
||||
+39
@@ -0,0 +1,39 @@
|
||||
From f47d4e3764f16acf05ad802843f80695f52b3652 Mon Sep 17 00:00:00 2001
|
||||
From: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
Date: Sun, 31 May 2026 17:25:08 +0200
|
||||
Subject: exec: prevent double close() of exec pipe descriptors
|
||||
|
||||
rpc_exec_process_cb() closes the stdout/stderr read descriptors after
|
||||
the child has exited, and rpc_exec_reply() closes them once more during
|
||||
teardown. ustream_free() neither closes nor resets the stored fd, so
|
||||
both code paths operate on the same descriptor number. In the window
|
||||
between the two closes the daemon may open another descriptor that
|
||||
reuses the freed number, which rpc_exec_reply() would then close by
|
||||
mistake.
|
||||
|
||||
Reset the descriptors to -1 after closing them in the process callback
|
||||
so the later close() in rpc_exec_reply() becomes a harmless no-op.
|
||||
|
||||
Assisted-by: Claude:claude-opus-4-8
|
||||
Link: https://github.com/openwrt/rpcd/pull/34
|
||||
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
(cherry picked from commit 26dba5206e1721efb217f6e7bafa27cd225bb698)
|
||||
---
|
||||
exec.c | 6 ++++++
|
||||
1 file changed, 6 insertions(+)
|
||||
|
||||
--- a/exec.c
|
||||
+++ b/exec.c
|
||||
@@ -203,6 +203,12 @@ rpc_exec_process_cb(struct uloop_process
|
||||
close(c->opipe.fd.fd);
|
||||
close(c->epipe.fd.fd);
|
||||
|
||||
+ /* ustream_free() does not reset the fd, and rpc_exec_reply() closes it
|
||||
+ * again later. Mark the descriptors as consumed so that the second
|
||||
+ * close() cannot accidentally close an unrelated, meanwhile reused fd. */
|
||||
+ c->opipe.fd.fd = -1;
|
||||
+ c->epipe.fd.fd = -1;
|
||||
+
|
||||
ustream_poll(&c->opipe.stream);
|
||||
ustream_poll(&c->epipe.stream);
|
||||
}
|
||||
+42
@@ -0,0 +1,42 @@
|
||||
From ee5070bce70a041ccbde011187cd33fb3fd5cf27 Mon Sep 17 00:00:00 2001
|
||||
From: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
Date: Sun, 31 May 2026 17:34:08 +0200
|
||||
Subject: uci: prevent integer overflow of client supplied apply timeout
|
||||
|
||||
rpc_uci_apply() reads the "timeout" argument as a uint32 into an int and
|
||||
scales it with "timeout * 1000" before passing it to
|
||||
uloop_timeout_set(). A large value overflows the signed multiplication,
|
||||
and a value with the high bit set wraps to a negative int; both cases
|
||||
produce a negative millisecond value that schedules the rollback timer
|
||||
in the past, firing it immediately instead of after the requested
|
||||
window.
|
||||
|
||||
Clamp the millisecond value to a sane range, consistent with the
|
||||
session timeout and -t argument overflow fixes.
|
||||
|
||||
Assisted-by: Claude:claude-opus-4-8
|
||||
Link: https://github.com/openwrt/rpcd/pull/34
|
||||
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
(cherry picked from commit af5d6f431186bcd1847e9d3210652b09dbc90ce5)
|
||||
---
|
||||
uci.c | 8 +++++++-
|
||||
1 file changed, 7 insertions(+), 1 deletion(-)
|
||||
|
||||
--- a/uci.c
|
||||
+++ b/uci.c
|
||||
@@ -1635,8 +1635,14 @@ rpc_uci_apply(struct ubus_context *ctx,
|
||||
globfree(&gl);
|
||||
|
||||
if (rollback) {
|
||||
+ /* Clamp to a sane range to avoid signed overflow of the
|
||||
+ * millisecond value for a large (or, via uint32 wraparound,
|
||||
+ * negative) client supplied timeout, which would otherwise
|
||||
+ * fire the rollback timer immediately. */
|
||||
+ int msecs = (timeout > 0 && timeout <= INT_MAX / 1000)
|
||||
+ ? timeout * 1000 : INT_MAX;
|
||||
apply_timer.cb = rpc_uci_apply_timeout;
|
||||
- uloop_timeout_set(&apply_timer, timeout * 1000);
|
||||
+ uloop_timeout_set(&apply_timer, msecs);
|
||||
apply_ctx = ctx;
|
||||
}
|
||||
}
|
||||
+113
@@ -0,0 +1,113 @@
|
||||
From 56a8ae88024efae44bfa296f3c8b01d2c350648f Mon Sep 17 00:00:00 2001
|
||||
From: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
Date: Sun, 31 May 2026 17:51:23 +0200
|
||||
Subject: treat exec failures in forked children with _exit() instead of return
|
||||
|
||||
Several handlers fork and, in the child, fall back to "return <ubus
|
||||
status>" when open()/malloc()/execv() etc. fail. Returning a ubus
|
||||
status code from the child does not terminate it: control unwinds back
|
||||
into the ubus dispatch loop and the child keeps running as a duplicate
|
||||
rpcd, racing the parent on the ubus socket and pipes.
|
||||
|
||||
Replace these child-side returns with _exit(127) so a child that cannot
|
||||
exec the target simply terminates, which is also what the parent's
|
||||
WEXITSTATUS based reporting expects. Affected paths: exec.c (rpc_exec),
|
||||
file.c (rpc_file_exec_run), sys.c (rpc_cgi_password_set) and plugin.c
|
||||
(rpc_plugin_register_exec).
|
||||
|
||||
Assisted-by: Claude:claude-opus-4-8
|
||||
Link: https://github.com/openwrt/rpcd/pull/34
|
||||
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
(cherry picked from commit 3037a0e36856159075624fb00b5410d5577ed5fe)
|
||||
---
|
||||
exec.c | 2 +-
|
||||
file.c | 10 +++++-----
|
||||
plugin.c | 2 +-
|
||||
sys.c | 4 ++--
|
||||
4 files changed, 9 insertions(+), 9 deletions(-)
|
||||
|
||||
--- a/exec.c
|
||||
+++ b/exec.c
|
||||
@@ -360,7 +360,7 @@ rpc_exec(const char **args, rpc_exec_wri
|
||||
close(epipe[1]);
|
||||
|
||||
if (execv(cmd, (char * const *)args))
|
||||
- return rpc_errno_status();
|
||||
+ _exit(127);
|
||||
|
||||
default:
|
||||
memset(c, 0, sizeof(*c));
|
||||
--- a/file.c
|
||||
+++ b/file.c
|
||||
@@ -896,7 +896,7 @@ rpc_file_exec_run(const char *cmd, const
|
||||
devnull = open("/dev/null", O_RDWR);
|
||||
|
||||
if (devnull == -1)
|
||||
- return UBUS_STATUS_UNKNOWN_ERROR;
|
||||
+ _exit(127);
|
||||
|
||||
dup2(devnull, 0);
|
||||
dup2(opipe[1], 1);
|
||||
@@ -912,7 +912,7 @@ rpc_file_exec_run(const char *cmd, const
|
||||
args = malloc(sizeof(char *) * arglen);
|
||||
|
||||
if (!args)
|
||||
- return UBUS_STATUS_UNKNOWN_ERROR;
|
||||
+ _exit(127);
|
||||
|
||||
args[0] = (char *)executable;
|
||||
args[1] = NULL;
|
||||
@@ -927,7 +927,7 @@ rpc_file_exec_run(const char *cmd, const
|
||||
if (arglen == 255)
|
||||
{
|
||||
free(args);
|
||||
- return UBUS_STATUS_INVALID_ARGUMENT;
|
||||
+ _exit(127);
|
||||
}
|
||||
|
||||
arglen++;
|
||||
@@ -936,7 +936,7 @@ rpc_file_exec_run(const char *cmd, const
|
||||
if (!tmp)
|
||||
{
|
||||
free(args);
|
||||
- return UBUS_STATUS_UNKNOWN_ERROR;
|
||||
+ _exit(127);
|
||||
}
|
||||
|
||||
args = tmp;
|
||||
@@ -957,7 +957,7 @@ rpc_file_exec_run(const char *cmd, const
|
||||
}
|
||||
|
||||
if (execv(executable, args))
|
||||
- return rpc_errno_status();
|
||||
+ _exit(127);
|
||||
|
||||
default:
|
||||
memset(c, 0, sizeof(*c));
|
||||
--- a/plugin.c
|
||||
+++ b/plugin.c
|
||||
@@ -451,7 +451,7 @@ rpc_plugin_register_exec(struct ubus_con
|
||||
close(fds[1]);
|
||||
|
||||
if (execl(path, path, "list", NULL))
|
||||
- return UBUS_STATUS_UNKNOWN_ERROR;
|
||||
+ _exit(127);
|
||||
|
||||
default:
|
||||
plugin = rpc_plugin_parse_exec(name + 1, fds[0]);
|
||||
--- a/sys.c
|
||||
+++ b/sys.c
|
||||
@@ -129,11 +129,11 @@ rpc_cgi_password_set(struct ubus_context
|
||||
|
||||
ret = chdir("/");
|
||||
if (ret < 0)
|
||||
- return rpc_errno_status();
|
||||
+ _exit(127);
|
||||
|
||||
if (execl(passwd, passwd,
|
||||
blobmsg_data(tb[RPC_P_USER]), NULL))
|
||||
- return rpc_errno_status();
|
||||
+ _exit(127);
|
||||
|
||||
default:
|
||||
close(fds[0]);
|
||||
+36
@@ -0,0 +1,36 @@
|
||||
From a47b5b9cbfcdbcc446ef411a0a594b76b2925ce2 Mon Sep 17 00:00:00 2001
|
||||
From: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
Date: Sun, 31 May 2026 18:08:25 +0200
|
||||
Subject: file: avoid zero-length b64_decode() on empty write data
|
||||
|
||||
rpc_file_write() computes data_len as blobmsg_data_len() - 1, which is 0
|
||||
for an empty "data" string. With base64 enabled it then called
|
||||
b64_decode(data, data, 0); b64_decode() asserts dest size > 0, so a
|
||||
request like file.write {"data":"","base64":true} aborts the daemon when
|
||||
libubox is built with assertions enabled (and is a pointless call
|
||||
otherwise).
|
||||
|
||||
Skip the decode when there is no data; the file is still created/
|
||||
truncated and zero bytes are written, which is the correct result.
|
||||
|
||||
Assisted-by: Claude:claude-opus-4-8
|
||||
Link: https://github.com/openwrt/rpcd/pull/34
|
||||
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
(cherry picked from commit 79c8087c8e8eb8933980fa6fc584703526388446)
|
||||
---
|
||||
file.c | 4 +++-
|
||||
1 file changed, 3 insertions(+), 1 deletion(-)
|
||||
|
||||
--- a/file.c
|
||||
+++ b/file.c
|
||||
@@ -405,7 +405,9 @@ rpc_file_write(struct ubus_context *ctx,
|
||||
if (fd < 0)
|
||||
return rpc_errno_status();
|
||||
|
||||
- if (tb[RPC_F_RW_BASE64] && blobmsg_get_bool(tb[RPC_F_RW_BASE64]))
|
||||
+ /* data_len can be 0 for an empty "data" string; skip the decode in that
|
||||
+ * case since b64_decode() asserts on a zero destination size. */
|
||||
+ if (data_len > 0 && tb[RPC_F_RW_BASE64] && blobmsg_get_bool(tb[RPC_F_RW_BASE64]))
|
||||
{
|
||||
data_len = b64_decode(data, data, data_len);
|
||||
if (data_len < 0)
|
||||
+103
@@ -0,0 +1,103 @@
|
||||
From af062601c6b2dc1da27a24d08cb8d11814cbc09e Mon Sep 17 00:00:00 2001
|
||||
From: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
Date: Sun, 31 May 2026 18:14:05 +0200
|
||||
Subject: ucode: bound recursion when converting blob arguments to ucode values
|
||||
|
||||
rpc_ucode_blob_array_to_ucv()/rpc_ucode_blob_to_ucv() recurse once per
|
||||
nesting level of the incoming request message, which is fully attacker
|
||||
controlled. ubus permits messages up to UBUS_MAX_MSGLEN (1 MiB) and
|
||||
libubox enforces no nesting limit, so a deeply nested array/table
|
||||
argument to a ucode plugin method can drive tens of thousands of
|
||||
recursion levels and overflow the stack, crashing rpcd.
|
||||
|
||||
Thread a depth counter through the conversion and stop descending past
|
||||
RPC_UCODE_MAX_NESTING (32) levels, which is far beyond any legitimate
|
||||
ubus message. Over-deep subtrees become null values instead of
|
||||
crashing.
|
||||
|
||||
Assisted-by: Claude:claude-opus-4-8
|
||||
Link: https://github.com/openwrt/rpcd/pull/34
|
||||
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
(cherry picked from commit cd1d9588da6368af1dbc99596ded90d7e5e027e1)
|
||||
---
|
||||
ucode.c | 26 ++++++++++++++++++--------
|
||||
1 file changed, 18 insertions(+), 8 deletions(-)
|
||||
|
||||
--- a/ucode.c
|
||||
+++ b/ucode.c
|
||||
@@ -34,6 +34,11 @@
|
||||
|
||||
#define RPC_UCSCRIPT_DIRECTORY INSTALL_PREFIX "/share/rpcd/ucode"
|
||||
|
||||
+/* Bound the recursion when converting a (client supplied) blob message into
|
||||
+ * ucode values, so a deeply nested argument cannot overflow the stack. Real
|
||||
+ * ubus messages never come anywhere near this depth. */
|
||||
+#define RPC_UCODE_MAX_NESTING 32
|
||||
+
|
||||
static struct blob_buf buf;
|
||||
static int request_timeout;
|
||||
|
||||
@@ -173,23 +178,28 @@ rpc_ucode_ucv_object_to_blob(uc_value_t
|
||||
}
|
||||
|
||||
static uc_value_t *
|
||||
-rpc_ucode_blob_to_ucv(uc_vm_t *vm, struct blob_attr *attr, bool table, const char **name);
|
||||
+rpc_ucode_blob_to_ucv(uc_vm_t *vm, struct blob_attr *attr, bool table, const char **name, int depth);
|
||||
|
||||
static uc_value_t *
|
||||
-rpc_ucode_blob_array_to_ucv(uc_vm_t *vm, struct blob_attr *attr, size_t len, bool table)
|
||||
+rpc_ucode_blob_array_to_ucv(uc_vm_t *vm, struct blob_attr *attr, size_t len, bool table, int depth)
|
||||
{
|
||||
- uc_value_t *o = table ? ucv_object_new(vm) : ucv_array_new(vm);
|
||||
+ uc_value_t *o;
|
||||
uc_value_t *v;
|
||||
struct blob_attr *pos;
|
||||
size_t rem = len;
|
||||
const char *name;
|
||||
|
||||
+ if (depth > RPC_UCODE_MAX_NESTING)
|
||||
+ return NULL;
|
||||
+
|
||||
+ o = table ? ucv_object_new(vm) : ucv_array_new(vm);
|
||||
+
|
||||
if (!o)
|
||||
return NULL;
|
||||
|
||||
__blob_for_each_attr(pos, attr, rem) {
|
||||
name = NULL;
|
||||
- v = rpc_ucode_blob_to_ucv(vm, pos, table, &name);
|
||||
+ v = rpc_ucode_blob_to_ucv(vm, pos, table, &name, depth);
|
||||
|
||||
if (table && name)
|
||||
ucv_object_add(o, name, v);
|
||||
@@ -203,7 +213,7 @@ rpc_ucode_blob_array_to_ucv(uc_vm_t *vm,
|
||||
}
|
||||
|
||||
static uc_value_t *
|
||||
-rpc_ucode_blob_to_ucv(uc_vm_t *vm, struct blob_attr *attr, bool table, const char **name)
|
||||
+rpc_ucode_blob_to_ucv(uc_vm_t *vm, struct blob_attr *attr, bool table, const char **name, int depth)
|
||||
{
|
||||
void *data;
|
||||
int len;
|
||||
@@ -245,10 +255,10 @@ rpc_ucode_blob_to_ucv(uc_vm_t *vm, struc
|
||||
return ucv_string_new(data);
|
||||
|
||||
case BLOBMSG_TYPE_ARRAY:
|
||||
- return rpc_ucode_blob_array_to_ucv(vm, data, len, false);
|
||||
+ return rpc_ucode_blob_array_to_ucv(vm, data, len, false, depth + 1);
|
||||
|
||||
case BLOBMSG_TYPE_TABLE:
|
||||
- return rpc_ucode_blob_array_to_ucv(vm, data, len, true);
|
||||
+ return rpc_ucode_blob_array_to_ucv(vm, data, len, true, depth + 1);
|
||||
|
||||
default:
|
||||
return NULL;
|
||||
@@ -313,7 +323,7 @@ rpc_ucode_validate_call_args(struct ubus
|
||||
}
|
||||
}
|
||||
|
||||
- *res = rpc_ucode_blob_array_to_ucv(&script->vm, blob_data(msg), blob_len(msg), true);
|
||||
+ *res = rpc_ucode_blob_array_to_ucv(&script->vm, blob_data(msg), blob_len(msg), true, 0);
|
||||
|
||||
return UBUS_STATUS_OK;
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
From 4591e70507777ac1bdd5cc3229b38db3a57da14e Mon Sep 17 00:00:00 2001
|
||||
From: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
Date: Sun, 31 May 2026 17:38:38 +0200
|
||||
Subject: rc: fix memory leak of list request context
|
||||
|
||||
rc_list() allocates a struct rc_list_context with calloc() and defers
|
||||
the request; rc_list_readdir() walks /etc/init.d asynchronously and,
|
||||
once the directory is exhausted (or the requested entry was found),
|
||||
sends the reply and completes the deferred request -- but never frees
|
||||
the context. free(c) only existed in the early opendir() failure path,
|
||||
so every successful "rc list" ubus call leaked the context (which embeds
|
||||
a PATH_MAX sized path buffer), slowly exhausting memory.
|
||||
|
||||
Free the context in the terminal path after completing the request.
|
||||
|
||||
Assisted-by: Claude:claude-opus-4-8
|
||||
Link: https://github.com/openwrt/rpcd/pull/34
|
||||
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
(cherry picked from commit d06d2a81dc29d746c5f13a9cc07ce779b5ed39be)
|
||||
---
|
||||
rc.c | 1 +
|
||||
1 file changed, 1 insertion(+)
|
||||
|
||||
--- a/rc.c
|
||||
+++ b/rc.c
|
||||
@@ -193,6 +193,7 @@ static void rc_list_readdir(struct rc_li
|
||||
closedir(c->dir);
|
||||
ubus_send_reply(c->ctx, &c->req, c->buf->head);
|
||||
ubus_complete_deferred_request(c->ctx, &c->req, UBUS_STATUS_OK);
|
||||
+ free(c);
|
||||
return;
|
||||
}
|
||||
|
||||
+57
@@ -0,0 +1,57 @@
|
||||
From 361104e320e1a5edd8fe52a28bad7bf193e83cc1 Mon Sep 17 00:00:00 2001
|
||||
From: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
Date: Sun, 31 May 2026 17:41:10 +0200
|
||||
Subject: rc: copy list "name" filter to avoid use-after-free
|
||||
|
||||
rc_list() stored the "name" argument as a bare pointer into the parsed
|
||||
request message and then deferred the request. The init.d directory is
|
||||
walked asynchronously across multiple uloop iterations (each script's
|
||||
"running" check is a forked child), during which the single per-context
|
||||
ubus receive buffer that backs the message is overwritten by any other
|
||||
incoming ubus request. c->req_name then points at unrelated/freed data
|
||||
and is compared against directory entries, leading to wrong matches or a
|
||||
crash.
|
||||
|
||||
Duplicate the name into the context and release it when the request
|
||||
completes.
|
||||
|
||||
Assisted-by: Claude:claude-opus-4-8
|
||||
Link: https://github.com/openwrt/rpcd/pull/34
|
||||
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
(cherry picked from commit 0de6668115593f2d316070573d12c29409c89078)
|
||||
---
|
||||
rc.c | 8 ++++++--
|
||||
1 file changed, 6 insertions(+), 2 deletions(-)
|
||||
|
||||
--- a/rc.c
|
||||
+++ b/rc.c
|
||||
@@ -50,7 +50,7 @@ struct rc_list_context {
|
||||
struct blob_buf *buf;
|
||||
DIR *dir;
|
||||
bool skip_running_check;
|
||||
- const char *req_name;
|
||||
+ char *req_name;
|
||||
|
||||
/* Info about currently processed init.d entry */
|
||||
struct {
|
||||
@@ -193,6 +193,7 @@ static void rc_list_readdir(struct rc_li
|
||||
closedir(c->dir);
|
||||
ubus_send_reply(c->ctx, &c->req, c->buf->head);
|
||||
ubus_complete_deferred_request(c->ctx, &c->req, UBUS_STATUS_OK);
|
||||
+ free(c->req_name);
|
||||
free(c);
|
||||
return;
|
||||
}
|
||||
@@ -283,8 +284,11 @@ static int rc_list(struct ubus_context *
|
||||
}
|
||||
if (tb[RC_LIST_SKIP_RUNNING_CHECK])
|
||||
c->skip_running_check = blobmsg_get_bool(tb[RC_LIST_SKIP_RUNNING_CHECK]);
|
||||
+ /* Copy the requested name: msg (and the ctx receive buffer it points
|
||||
+ * into) is only valid during this call, but req_name is dereferenced
|
||||
+ * later from the deferred, asynchronous directory walk. */
|
||||
if (tb[RC_LIST_NAME])
|
||||
- c->req_name = blobmsg_get_string(tb[RC_LIST_NAME]);
|
||||
+ c->req_name = strdup(blobmsg_get_string(tb[RC_LIST_NAME]));
|
||||
|
||||
ubus_defer_request(ctx, req, &c->req);
|
||||
|
||||
+97
@@ -0,0 +1,97 @@
|
||||
From 3e026930ab4bbf6d741e9586d5ec207a6e6496d4 Mon Sep 17 00:00:00 2001
|
||||
From: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
Date: Sun, 31 May 2026 17:42:56 +0200
|
||||
Subject: rc: use a per-request blob_buf for the list reply
|
||||
|
||||
rc_list() built the reply in a function-local "static struct blob_buf",
|
||||
shared by every invocation, and kept using it across the deferred,
|
||||
asynchronous directory walk. Two overlapping "rc list" requests (the
|
||||
walk waits on forked children, so a second request is easily interleaved)
|
||||
would both blob_buf_init() and append to the same buffer, corrupting
|
||||
each other's reply or crashing.
|
||||
|
||||
Move the blob_buf into the per-request context so each request owns its
|
||||
own buffer, and free it when the request completes.
|
||||
|
||||
Assisted-by: Claude:claude-opus-4-8
|
||||
Link: https://github.com/openwrt/rpcd/pull/34
|
||||
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
(cherry picked from commit 75470f4b5124074ff078ed7052bb756e3b694712)
|
||||
---
|
||||
rc.c | 24 ++++++++++++------------
|
||||
1 file changed, 12 insertions(+), 12 deletions(-)
|
||||
|
||||
--- a/rc.c
|
||||
+++ b/rc.c
|
||||
@@ -47,7 +47,7 @@ struct rc_list_context {
|
||||
struct uloop_timeout timeout;
|
||||
struct ubus_context *ctx;
|
||||
struct ubus_request_data req;
|
||||
- struct blob_buf *buf;
|
||||
+ struct blob_buf buf;
|
||||
DIR *dir;
|
||||
bool skip_running_check;
|
||||
char *req_name;
|
||||
@@ -88,17 +88,17 @@ static void rc_list_add_table(struct rc_
|
||||
{
|
||||
void *e;
|
||||
|
||||
- e = blobmsg_open_table(c->buf, c->entry.d_name);
|
||||
+ e = blobmsg_open_table(&c->buf, c->entry.d_name);
|
||||
|
||||
if (c->entry.start >= 0)
|
||||
- blobmsg_add_u16(c->buf, "start", c->entry.start);
|
||||
+ blobmsg_add_u16(&c->buf, "start", c->entry.start);
|
||||
if (c->entry.stop >= 0)
|
||||
- blobmsg_add_u16(c->buf, "stop", c->entry.stop);
|
||||
- blobmsg_add_u8(c->buf, "enabled", c->entry.enabled);
|
||||
+ blobmsg_add_u16(&c->buf, "stop", c->entry.stop);
|
||||
+ blobmsg_add_u8(&c->buf, "enabled", c->entry.enabled);
|
||||
if (!c->skip_running_check && c->entry.use_procd)
|
||||
- blobmsg_add_u8(c->buf, "running", c->entry.running);
|
||||
+ blobmsg_add_u8(&c->buf, "running", c->entry.running);
|
||||
|
||||
- blobmsg_close_table(c->buf, e);
|
||||
+ blobmsg_close_table(&c->buf, e);
|
||||
}
|
||||
|
||||
static void rpc_list_exec_timeout_cb(struct uloop_timeout *t)
|
||||
@@ -191,8 +191,9 @@ static void rc_list_readdir(struct rc_li
|
||||
*/
|
||||
if (!e || (c->req_name && c->entry.d_name)) {
|
||||
closedir(c->dir);
|
||||
- ubus_send_reply(c->ctx, &c->req, c->buf->head);
|
||||
+ ubus_send_reply(c->ctx, &c->req, c->buf.head);
|
||||
ubus_complete_deferred_request(c->ctx, &c->req, UBUS_STATUS_OK);
|
||||
+ blob_buf_free(&c->buf);
|
||||
free(c->req_name);
|
||||
free(c);
|
||||
return;
|
||||
@@ -264,24 +265,23 @@ static int rc_list(struct ubus_context *
|
||||
struct blob_attr *msg)
|
||||
{
|
||||
struct blob_attr *tb[__RC_LIST_MAX];
|
||||
- static struct blob_buf buf;
|
||||
struct rc_list_context *c;
|
||||
|
||||
blobmsg_parse(rc_list_policy, __RC_LIST_MAX, tb, blobmsg_data(msg), blobmsg_data_len(msg));
|
||||
|
||||
- blob_buf_init(&buf, 0);
|
||||
-
|
||||
c = calloc(1, sizeof(*c));
|
||||
if (!c)
|
||||
return UBUS_STATUS_UNKNOWN_ERROR;
|
||||
|
||||
c->ctx = ctx;
|
||||
- c->buf = &buf;
|
||||
c->dir = opendir("/etc/init.d");
|
||||
if (!c->dir) {
|
||||
free(c);
|
||||
return UBUS_STATUS_UNKNOWN_ERROR;
|
||||
}
|
||||
+
|
||||
+ blob_buf_init(&c->buf, 0);
|
||||
+
|
||||
if (tb[RC_LIST_SKIP_RUNNING_CHECK])
|
||||
c->skip_running_check = blobmsg_get_bool(tb[RC_LIST_SKIP_RUNNING_CHECK]);
|
||||
/* Copy the requested name: msg (and the ctx receive buffer it points
|
||||
+275
@@ -0,0 +1,275 @@
|
||||
From 25ca51acb9ce43c438252bd1cb5cf7f12f5e7f0c Mon Sep 17 00:00:00 2001
|
||||
From: Jo-Philipp Wich <jo@mein.io>
|
||||
Date: Sun, 19 Jul 2026 14:57:18 +0200
|
||||
Subject: file: re-authorize ACL against resolved path to close symlink bypass
|
||||
|
||||
The file plugin matched ACL grants against the textual, canonicalized
|
||||
path but then let open()/stat()/opendir() follow symlinks unchecked.
|
||||
A symlink placed inside an ACL-covered directory therefore let a grant
|
||||
on the link authorize read/write/list/stat access to whatever file it
|
||||
pointed to, including files entirely outside the granted scope.
|
||||
|
||||
Re-resolve the path with realpath() and re-run the ACL check against
|
||||
the resolved target whenever it differs from the canonicalized string,
|
||||
for every operation that dereferences the final component (read,
|
||||
write, md5, list, stat). lstat and remove are left untouched since
|
||||
they either need to see the link itself or already handle it safely
|
||||
via unlink()'s no-follow semantics. Newly created files (file.write)
|
||||
and dangling symlinks pointing outside the granted scope are handled
|
||||
via the containing directory.
|
||||
|
||||
Also mask file.write's mode parameter to 0777 and drop the local
|
||||
umask(0) override, and authorize recursive file.remove per-entry
|
||||
instead of only at the top-level path.
|
||||
|
||||
Fixes GHSA-q5gr-86pq-vvwr.
|
||||
|
||||
Signed-off-by: Jo-Philipp Wich <jo@mein.io>
|
||||
(cherry picked from commit e37ed9d814699098eb7e26c8b33c054840782dfb)
|
||||
---
|
||||
file.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
|
||||
1 file changed, 144 insertions(+), 12 deletions(-)
|
||||
|
||||
--- a/file.c
|
||||
+++ b/file.c
|
||||
@@ -77,6 +77,7 @@ struct rpc_file_exec_context {
|
||||
|
||||
static struct blob_buf buf;
|
||||
static char *canonpath;
|
||||
+static char *resolvedpath;
|
||||
static char cmdstr[RPC_CMDLINE_MAX_SIZE];
|
||||
|
||||
enum {
|
||||
@@ -246,10 +247,122 @@ next:
|
||||
return canonpath;
|
||||
}
|
||||
|
||||
+/*
|
||||
+ * rpc_canonicalize_path() only folds "//", "/./" and "/../" textually; it
|
||||
+ * never resolves symlink components. Since the ACL check above matches the
|
||||
+ * textual path, a symlink placed inside an ACL-covered directory would let
|
||||
+ * a grant on the link authorize whatever file the link points to once the
|
||||
+ * caller's stat()/open()/opendir() follows it. Re-resolve *path with
|
||||
+ * realpath() and, if that changes the path, re-run the ACL check against
|
||||
+ * the resolved target so grants only ever cover the real file. If the
|
||||
+ * target does not exist yet (e.g. a new file being written), resolve and
|
||||
+ * re-check the containing directory instead and rebuild *path from that.
|
||||
+ */
|
||||
+static bool
|
||||
+rpc_check_symlink_access(const struct blob_attr *sid, const char *perm, char **path)
|
||||
+{
|
||||
+ char resolved[PATH_MAX];
|
||||
+ char dirbuf[PATH_MAX];
|
||||
+ const char *base;
|
||||
+ struct stat lst;
|
||||
+
|
||||
+ if (realpath(*path, resolved) != NULL)
|
||||
+ {
|
||||
+ if (strcmp(resolved, *path) == 0)
|
||||
+ return true;
|
||||
+
|
||||
+ if (!rpc_file_access(sid, resolved, perm))
|
||||
+ {
|
||||
+ errno = EACCES;
|
||||
+ return false;
|
||||
+ }
|
||||
+
|
||||
+ free(resolvedpath);
|
||||
+ resolvedpath = strdup(resolved);
|
||||
+
|
||||
+ if (resolvedpath == NULL)
|
||||
+ {
|
||||
+ errno = ENOMEM;
|
||||
+ return false;
|
||||
+ }
|
||||
+
|
||||
+ *path = resolvedpath;
|
||||
+ return true;
|
||||
+ }
|
||||
+
|
||||
+ if (errno == ENOENT)
|
||||
+ {
|
||||
+ /* realpath() also fails with ENOENT for a dangling symlink whose
|
||||
+ * final target component is missing. Distinguish that case (an
|
||||
+ * *existing* symlink we must not silently create-through, e.g.
|
||||
+ * via open(O_CREAT) on file.write) from a genuinely nonexistent
|
||||
+ * path by lstat()'ing the requested path itself. */
|
||||
+ errno = (lstat(*path, &lst) == 0 && S_ISLNK(lst.st_mode)) ? EACCES : 0;
|
||||
+ }
|
||||
+
|
||||
+ if (errno != 0)
|
||||
+ return false;
|
||||
+
|
||||
+ base = strrchr(*path, '/');
|
||||
+
|
||||
+ if (base == NULL)
|
||||
+ {
|
||||
+ errno = ENOENT;
|
||||
+ return false;
|
||||
+ }
|
||||
+
|
||||
+ if (base == *path)
|
||||
+ {
|
||||
+ dirbuf[0] = '/';
|
||||
+ dirbuf[1] = '\0';
|
||||
+ }
|
||||
+ else if ((size_t)(base - *path) >= sizeof(dirbuf))
|
||||
+ {
|
||||
+ errno = ENAMETOOLONG;
|
||||
+ return false;
|
||||
+ }
|
||||
+ else
|
||||
+ {
|
||||
+ memcpy(dirbuf, *path, base - *path);
|
||||
+ dirbuf[base - *path] = '\0';
|
||||
+ }
|
||||
+
|
||||
+ base++;
|
||||
+
|
||||
+ if (realpath(dirbuf, resolved) == NULL)
|
||||
+ return false;
|
||||
+
|
||||
+ if (strcmp(resolved, dirbuf) == 0)
|
||||
+ {
|
||||
+ errno = ENOENT;
|
||||
+ return true;
|
||||
+ }
|
||||
+
|
||||
+ free(resolvedpath);
|
||||
+
|
||||
+ if (asprintf(&resolvedpath, "%s/%s", resolved, base) < 0)
|
||||
+ {
|
||||
+ errno = ENOMEM;
|
||||
+ return false;
|
||||
+ }
|
||||
+
|
||||
+ if (!rpc_file_access(sid, resolvedpath, perm))
|
||||
+ {
|
||||
+ errno = EACCES;
|
||||
+ return false;
|
||||
+ }
|
||||
+
|
||||
+ *path = resolvedpath;
|
||||
+ errno = ENOENT;
|
||||
+
|
||||
+ return true;
|
||||
+}
|
||||
+
|
||||
static struct blob_attr **
|
||||
__rpc_check_path(const struct blobmsg_policy *policy, size_t policy_len,
|
||||
int policy_path_idx, int policy_sid_idx, const char *perm,
|
||||
- struct blob_attr *msg, char **path, struct stat *s)
|
||||
+ struct blob_attr *msg, char **path, struct stat *s,
|
||||
+ bool resolve_symlinks)
|
||||
{
|
||||
static struct blob_attr *tb[__RPC_F_RW_MAX]; /* largest _MAX constant */
|
||||
|
||||
@@ -275,18 +388,32 @@ __rpc_check_path(const struct blobmsg_po
|
||||
return NULL;
|
||||
}
|
||||
|
||||
+ if (resolve_symlinks && !rpc_check_symlink_access(tb[policy_sid_idx], perm, path))
|
||||
+ return NULL;
|
||||
+
|
||||
if (s != NULL && stat(*path, s) != 0)
|
||||
return NULL;
|
||||
|
||||
return tb;
|
||||
}
|
||||
|
||||
+// symlinks are resolved and the ACL re-checked against the resolved target
|
||||
#define rpc_check_path(msg, policy_selector, perm, path, s) \
|
||||
__rpc_check_path(rpc_file_ ## policy_selector ## _policy, \
|
||||
ARRAY_SIZE(rpc_file_ ## policy_selector ## _policy), \
|
||||
RPC_F_ ## policy_selector ## _PATH, \
|
||||
RPC_F_ ## policy_selector ## _SESSION, \
|
||||
- perm, msg, path, s)
|
||||
+ perm, msg, path, s, true)
|
||||
+
|
||||
+// symlinks are left unresolved since the caller either wants to inspect the
|
||||
+// link itself or already handles it safely via unlink()'s no-follow
|
||||
+// semantics (remove)
|
||||
+#define rpc_check_path_no_resolve(msg, policy_selector, perm, path, s) \
|
||||
+ __rpc_check_path(rpc_file_ ## policy_selector ## _policy, \
|
||||
+ ARRAY_SIZE(rpc_file_ ## policy_selector ## _policy), \
|
||||
+ RPC_F_ ## policy_selector ## _PATH, \
|
||||
+ RPC_F_ ## policy_selector ## _SESSION, \
|
||||
+ perm, msg, path, s, false)
|
||||
|
||||
static int
|
||||
rpc_file_read(struct ubus_context *ctx, struct ubus_object *obj,
|
||||
@@ -376,7 +503,7 @@ rpc_file_write(struct ubus_context *ctx,
|
||||
{
|
||||
struct blob_attr **tb;
|
||||
int append = O_TRUNC;
|
||||
- mode_t prev_mode, mode = 0666;
|
||||
+ mode_t mode = 0666;
|
||||
int fd, rv = 0;
|
||||
char *path = NULL;
|
||||
void *data = NULL;
|
||||
@@ -397,11 +524,9 @@ rpc_file_write(struct ubus_context *ctx,
|
||||
append = O_APPEND;
|
||||
|
||||
if (tb[RPC_F_RW_MODE])
|
||||
- mode = blobmsg_get_u32(tb[RPC_F_RW_MODE]);
|
||||
+ mode = blobmsg_get_u32(tb[RPC_F_RW_MODE]) & 0777;
|
||||
|
||||
- prev_mode = umask(0);
|
||||
fd = open(path, O_CREAT | O_WRONLY | append, mode);
|
||||
- umask(prev_mode);
|
||||
if (fd < 0)
|
||||
return rpc_errno_status();
|
||||
|
||||
@@ -562,10 +687,10 @@ rpc_file_stat(struct ubus_context *ctx,
|
||||
}
|
||||
|
||||
static int
|
||||
-rpc_file_remove_recursive(const char *path);
|
||||
+rpc_file_remove_recursive(const struct blob_attr *sid, const char *path);
|
||||
|
||||
static int
|
||||
-rpc_file_remove_recursive(const char *path)
|
||||
+rpc_file_remove_recursive(const struct blob_attr *sid, const char *path)
|
||||
{
|
||||
DIR *fd;
|
||||
int err = 0;
|
||||
@@ -583,10 +708,14 @@ rpc_file_remove_recursive(const char *pa
|
||||
|
||||
if (asprintf(&entrypath, "%s/%s", path, e->d_name) >= 0)
|
||||
{
|
||||
- if (!lstat(entrypath, &s))
|
||||
+ if (!rpc_file_access(sid, entrypath, "write"))
|
||||
+ {
|
||||
+ err = UBUS_STATUS_PERMISSION_DENIED;
|
||||
+ }
|
||||
+ else if (!lstat(entrypath, &s))
|
||||
{
|
||||
if (S_ISDIR(s.st_mode))
|
||||
- err = rpc_file_remove_recursive(entrypath);
|
||||
+ err = rpc_file_remove_recursive(sid, entrypath);
|
||||
else if (unlink(entrypath))
|
||||
err = rpc_errno_status();
|
||||
}
|
||||
@@ -612,17 +741,20 @@ rpc_file_remove(struct ubus_context *ctx
|
||||
struct ubus_request_data *req, const char *method,
|
||||
struct blob_attr *msg)
|
||||
{
|
||||
+ struct blob_attr **tb;
|
||||
struct stat s;
|
||||
char *path = NULL;
|
||||
|
||||
- if (!rpc_check_path(msg, R, "write", &path, NULL))
|
||||
+ tb = rpc_check_path_no_resolve(msg, R, "write", &path, NULL);
|
||||
+
|
||||
+ if (tb == NULL)
|
||||
return rpc_errno_status();
|
||||
|
||||
if (lstat(path, &s))
|
||||
return rpc_errno_status();
|
||||
|
||||
if (S_ISDIR(s.st_mode))
|
||||
- return rpc_file_remove_recursive(path);
|
||||
+ return rpc_file_remove_recursive(tb[RPC_F_R_SESSION], path);
|
||||
|
||||
if (unlink(path))
|
||||
return rpc_errno_status();
|
||||
Reference in new issue
Block a user