From: Juergen Perlinger <perlinger@ntp.org>
Date: Sun, 30 Apr 2023 06:54:37 +0000 (+0200)
Subject: [Bug 3808] Assertion failure in ntpq on malformed RT-11 date
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e63f69876c589482e76a5844db946de27a4193ca;p=thirdparty%2Fntp.git

[Bug 3808] Assertion failure in ntpq on malformed RT-11 date

bk: 644e10adV0TM_-yj-L713VY5YsZcAQ
---

diff --git a/ChangeLog b/ChangeLog
index 23c6a6c95..4f8b7c360 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,5 @@
 ---
+* [Bug 3808] Assertion failure in ntpq on malformed RT-11 date <perlinger@ntp.org>
 * [Bug 3802] ntp-keygen -I default identity modulus bits too small for
              OpenSSL 3.  Reported by rmsh1216@163.com <hart@ntp.org>
 * [Bug 3801] gpsdjson refclock gps_open() device name mishandled. <hart@ntp.org>
diff --git a/libntp/caltontp.c b/libntp/caltontp.c
index 3fb5390db..b0de04b18 100644
--- a/libntp/caltontp.c
+++ b/libntp/caltontp.c
@@ -17,13 +17,13 @@
  * otherwise a full turn through the calendar calculations will be taken.
  *
  * I know that Harlan Stenn likes to see assertions in production code, and I
- * agree there, but it would be a tricky thing here. The algorithm is quite
- * capable of producing sensible answers even to seemingly weird inputs: the
- * date <any year here>-03-00, the 0.th March of the year, will be automtically
- * treated as the last day of February, no matter whether the year is a leap
- * year or not. So adding constraints is merely for the benefit of the callers,
- * because the only thing we can check for consistency is our input, produced
- * by somebody else.
+ * agree in general. But here we set 'errno' and try to do our best instead.
+ * Also note that the bounds check is a bit sloppy: It permits off-by-one
+ * on the input quantities. That permits some simple/naive adjustments to
+ * be made before calling this function.
+ *
+ * Apart from that the calendar is perfectly capable of dealing with
+ * off-scale input values!
  *
  * BTW: A total roundtrip using 'caljulian' would be a quite shaky thing:
  * Because of the truncation of the NTP time stamp to 32 bits and the epoch
@@ -40,14 +40,18 @@ caltontp(
 	int32_t eraday;	/* CE Rata Die number	*/
 	vint64  ntptime;/* resulting NTP time	*/
 
-	REQUIRE(jt != NULL);
+	if (NULL == jt) {
+		errno = EINVAL;
+		return 0;
+	}
 
-	REQUIRE(jt->month <= 13);	/* permit month 0..13! */
-	REQUIRE(jt->monthday <= 32);
-	REQUIRE(jt->yearday <= 366);
-	REQUIRE(jt->hour <= 24);
-	REQUIRE(jt->minute <= MINSPERHR);
-	REQUIRE(jt->second <= SECSPERMIN);
+	if (   (jt->month > 13)	/* permit month 0..13! */
+	    || (jt->monthday > 32)
+	    || (jt->yearday > 366)
+	    || (jt->hour > 24)
+	    || (jt->minute > MINSPERHR)
+	    || (jt->second > SECSPERMIN))
+		errno = ERANGE;
 
 	/*
 	 * First convert the date to he corresponding RataDie
diff --git a/ntpq/ntpq.c b/ntpq/ntpq.c
index 9d48e124f..1a0e9b1a6 100644
--- a/ntpq/ntpq.c
+++ b/ntpq/ntpq.c
@@ -2207,9 +2207,11 @@ rtdatetolfp(
 	if (cal.year < 100)
 		cal.year += 1900;
 
-	lfp->l_ui = caltontp(&cal);
+	/* check for complaints from 'caltontp()'! */
 	lfp->l_uf = 0;
-	return 1;
+	errno = 0;
+	lfp->l_ui = caltontp(&cal);
+	return (errno == 0);
 }