session: clamp uloop timeout to avoid int overflow#30
session: clamp uloop timeout to avoid int overflow#30Scared-Heart wants to merge 1 commit intoopenwrt:masterfrom
Conversation
Summary
The same overflow exists in User-visible impactAny LuCI's ucode dispatcher ( At least one affected user had to factory-reset several APs before recovering — see forum thread: https://forum.openwrt.org/t/how-to-set-session-timeout-without-having-to-reset-and-reconfigure/241892. Reproduction
FixIntroduce a small helper The cap still permits ~24.85 days of idle timeout, longer than any realistic administrative session. Values larger than that are silently capped rather than rejected, preserving current API behavior for existing callers. Test plan
|
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>
380fbc4 to
d005c88
Compare
Fixes #29
rpc_touch_session() computes
ses->timeout * 1000as 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 loginwith e.g.timeout:2592000(30 days) return a valid-looking session id whose reportedexpiresis 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 ofluci.sauth.sessiontimestraight through as the login timeout, so users who follow common advice to bump sessiontime to 30 days get silently locked out of LuCI (uhttpd logsaccepted 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) * 1000is passed to uloop_timeout_set()'s intmsecsparameter.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.