From e39de45adc435629ef8925edc5022bf15c8971a3 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 8 Mar 2024 12:12:50 +0100
Subject: [PATCH 1/2] Detect invalid durations

Be stricter in durations that are accepted. Basically we accept ISO 8601
formats, but fail to detect garbage after the integers in such strings.

For example, 'P7.5D' will be treated as 7 days. Pass 'endptr' to
'strtoll' and check if the endptr is at the correct suffix.
---
 .../system/checkconf/bad-kasp-duration.conf   | 25 +++++++++++++
 lib/isccfg/duration.c                         | 37 +++++++++++++++----
 2 files changed, 55 insertions(+), 7 deletions(-)
 create mode 100644 bin/tests/system/checkconf/bad-kasp-duration.conf

diff --git a/bin/tests/system/checkconf/bad-kasp-duration.conf b/bin/tests/system/checkconf/bad-kasp-duration.conf
new file mode 100644
index 0000000000..74f08271b7
--- /dev/null
+++ b/bin/tests/system/checkconf/bad-kasp-duration.conf
@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0.  If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+dnssec-policy "invalid-sigrefresh" {
+	keys {
+		csk lifetime unlimited algorithm 13;
+	};
+	signatures-refresh P7.5D;
+};
+
+zone "example.net" {
+	type primary;
+	file "example.db";
+	dnssec-policy "invalid-sigrefresh";
+};
diff --git a/lib/isccfg/duration.c b/lib/isccfg/duration.c
index edb8370f40..ee1c570786 100644
--- a/lib/isccfg/duration.c
+++ b/lib/isccfg/duration.c
@@ -44,6 +44,7 @@ isccfg_duration_fromtext(isc_textregion_t *source,
 	bool not_weeks = false;
 	int i;
 	long long int lli;
+	char *endptr;
 
 	/*
 	 * Copy the buffer as it may not be NULL terminated.
@@ -75,7 +76,11 @@ isccfg_duration_fromtext(isc_textregion_t *source,
 	X = strpbrk(str, "Yy");
 	if (X != NULL) {
 		errno = 0;
-		lli = strtoll(str + 1, NULL, 10);
+		endptr = NULL;
+		lli = strtoll(str + 1, &endptr, 10);
+		if (*endptr != *X) {
+			return (ISC_R_BADNUMBER);
+		}
 		if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
 			return (ISC_R_BADNUMBER);
 		}
@@ -93,7 +98,10 @@ isccfg_duration_fromtext(isc_textregion_t *source,
 	 */
 	if (X != NULL && (T == NULL || (size_t)(X - P) < (size_t)(T - P))) {
 		errno = 0;
-		lli = strtoll(str + 1, NULL, 10);
+		lli = strtoll(str + 1, &endptr, 10);
+		if (*endptr != *X) {
+			return (ISC_R_BADNUMBER);
+		}
 		if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
 			return (ISC_R_BADNUMBER);
 		}
@@ -106,7 +114,10 @@ isccfg_duration_fromtext(isc_textregion_t *source,
 	X = strpbrk(str, "Dd");
 	if (X != NULL) {
 		errno = 0;
-		lli = strtoll(str + 1, NULL, 10);
+		lli = strtoll(str + 1, &endptr, 10);
+		if (*endptr != *X) {
+			return (ISC_R_BADNUMBER);
+		}
 		if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
 			return (ISC_R_BADNUMBER);
 		}
@@ -125,7 +136,10 @@ isccfg_duration_fromtext(isc_textregion_t *source,
 	X = strpbrk(str, "Hh");
 	if (X != NULL && T != NULL) {
 		errno = 0;
-		lli = strtoll(str + 1, NULL, 10);
+		lli = strtoll(str + 1, &endptr, 10);
+		if (*endptr != *X) {
+			return (ISC_R_BADNUMBER);
+		}
 		if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
 			return (ISC_R_BADNUMBER);
 		}
@@ -143,7 +157,10 @@ isccfg_duration_fromtext(isc_textregion_t *source,
 	 */
 	if (X != NULL && T != NULL && (size_t)(X - P) > (size_t)(T - P)) {
 		errno = 0;
-		lli = strtoll(str + 1, NULL, 10);
+		lli = strtoll(str + 1, &endptr, 10);
+		if (*endptr != *X) {
+			return (ISC_R_BADNUMBER);
+		}
 		if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
 			return (ISC_R_BADNUMBER);
 		}
@@ -156,7 +173,10 @@ isccfg_duration_fromtext(isc_textregion_t *source,
 	X = strpbrk(str, "Ss");
 	if (X != NULL && T != NULL) {
 		errno = 0;
-		lli = strtoll(str + 1, NULL, 10);
+		lli = strtoll(str + 1, &endptr, 10);
+		if (*endptr != *X) {
+			return (ISC_R_BADNUMBER);
+		}
 		if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
 			return (ISC_R_BADNUMBER);
 		}
@@ -173,7 +193,10 @@ isccfg_duration_fromtext(isc_textregion_t *source,
 			return (ISC_R_BADNUMBER);
 		} else {
 			errno = 0;
-			lli = strtoll(str + 1, NULL, 10);
+			lli = strtoll(str + 1, &endptr, 10);
+			if (*endptr != *W) {
+				return (ISC_R_BADNUMBER);
+			}
 			if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
 				return (ISC_R_BADNUMBER);
 			}

From bc600ae2a17d6338a4e4ee7f500cf0fc47a0ea51 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 8 Mar 2024 12:23:40 +0100
Subject: [PATCH 2/2] Add CHANGES and release note for #4624

---
 CHANGES                     | 3 +++
 doc/notes/notes-current.rst | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/CHANGES b/CHANGES
index 1c3460e703..d2cda38462 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+6361.	[bug]		Some invalid ISO 8601 durations were accepted
+			erroneously. [GL #4624]
+
 6360.	[bug]		Don't return static-stub synthesised NS RRset.
 			[GL #4608]
 
diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst
index f94aaf3650..3ed4747884 100644
--- a/doc/notes/notes-current.rst
+++ b/doc/notes/notes-current.rst
@@ -37,6 +37,9 @@ Bug Fixes
 
 - None.
 
+- Some ISO 8601 durations were accepted erroneously, leading to shorter
+  durations than expected. This has been fixed. :gl:`#4624`
+
 Known Issues
 ~~~~~~~~~~~~