heya,
* [email protected] <[email protected]> [2025-08-07 01:02]:
> We have found several privsep interface bugs in `ntpd`, where the unprivileged
> `ntp` compartment can trigger undefined behavior inside the privileged
> compartment. We think that these bugs have *no security impact* but are still
> worth fixing, thus sending this report to bugs@.
absolutely.
> The privileged compartment accepts multiple messages from the unprivileged
> `ntp` compartment, where the payload is a `double` arithmetic value. We
> identified several cases where the privileged parent performs a lossy
> conversion from `double` to `long` to extract the integer part of the
> `double`.
> This conversion is well-defined only when the `double` value is between
> `LONG_MIN` and `LONG_MAX`. However, the privileged compartment does not verify
> whether the `double` is in this range before the conversion to a `long`,
> allowing the unprivileged `ntp` compartment to send a value outside this
> range,
> thus triggering undefined behavior inside the privileged compartment.
I guess the most sensible approach is to give d_to_tv a return value
and make use of it.
the log_warn in ntpd_settime could also be a fatal, after all, but the
existing ones are also ust warnings.
thanks for finding, analysis and report!
cheers
henning
Index: ntpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/ntpd/ntpd.c,v
diff -u -p -r1.142 ntpd.c
--- ntpd.c 21 Nov 2024 13:38:14 -0000 1.142
+++ ntpd.c 8 Aug 2025 13:08:35 -0000
@@ -410,6 +410,8 @@ dispatch_imsg(struct ntpd_conf *lconf, i
fatalx("invalid IMSG_ADJTIME received");
memcpy(&d, imsg.data, sizeof(d));
n = ntpd_adjtime(d);
+ if (n == -1)
+ fatalx("IMSG_ADJTIME with invalid value");
imsg_compose(ibuf, IMSG_ADJTIME, 0, 0, -1,
&n, sizeof(n));
break;
@@ -474,7 +476,8 @@ ntpd_adjtime(double d)
log_info("adjusting local clock by %fs", d);
else
log_debug("adjusting local clock by %fs", d);
- d_to_tv(d, &tv);
+ if (d_to_tv(d, &tv) == -1)
+ return (-1);
if (adjtime(&tv, &olddelta) == -1)
log_warn("adjtime failed");
else if (!firstadj && olddelta.tv_sec == 0 && olddelta.tv_usec == 0)
@@ -532,7 +535,10 @@ ntpd_settime(double d)
log_warn("gettimeofday");
return;
}
- d_to_tv(d, &tv);
+ if (d_to_tv(d, &tv) == -1) {
+ log_warn("ntpd_settime: invalid value");
+ return;
+ }
curtime.tv_usec += tv.tv_usec + 1000000;
curtime.tv_sec += tv.tv_sec - 1 + (curtime.tv_usec / 1000000);
curtime.tv_usec %= 1000000;
Index: ntpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/ntpd/ntpd.h,v
diff -u -p -r1.154 ntpd.h
--- ntpd.h 21 May 2024 05:00:48 -0000 1.154
+++ ntpd.h 8 Aug 2025 13:08:35 -0000
@@ -399,7 +399,7 @@ double gettime_from_timeval(struct ti
double getoffset(void);
double gettime(void);
time_t getmonotime(void);
-void d_to_tv(double, struct timeval *);
+int d_to_tv(double, struct timeval *);
double lfp_to_d(struct l_fixedpt);
struct l_fixedpt d_to_lfp(double);
double sfp_to_d(struct s_fixedpt);
Index: util.c
===================================================================
RCS file: /cvs/src/usr.sbin/ntpd/util.c,v
diff -u -p -r1.28 util.c
--- util.c 20 Dec 2023 15:36:36 -0000 1.28
+++ util.c 8 Aug 2025 13:08:35 -0000
@@ -73,15 +73,20 @@ getmonotime(void)
}
-void
+int
d_to_tv(double d, struct timeval *tv)
{
+ if (d > (double)LONG_MAX || d < (double)LONG_MIN)
+ return (-1);
+
tv->tv_sec = d;
tv->tv_usec = (d - tv->tv_sec) * 1000000;
while (tv->tv_usec < 0) {
tv->tv_usec += 1000000;
tv->tv_sec -= 1;
}
+
+ return (0);
}
double
--
Henning Brauer, [email protected], [email protected]
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS. Virtual & Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/
