From a621e035d4570485307762de37db456f3b249460 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking 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. (cherry picked from commit e39de45adc435629ef8925edc5022bf15c8971a3) --- .../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 9ed9d6f657..98476a085b 100644 --- a/lib/isccfg/duration.c +++ b/lib/isccfg/duration.c @@ -45,6 +45,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. @@ -76,7 +77,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); } @@ -94,7 +99,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); } @@ -107,7 +115,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); } @@ -126,7 +137,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); } @@ -144,7 +158,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); } @@ -157,7 +174,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); } @@ -174,7 +194,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 349b666b3f10a6d3b7ac94124eda7cdb488d480f Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Fri, 8 Mar 2024 12:23:40 +0100 Subject: [PATCH 2/2] Add CHANGES and release note for #4624 (cherry picked from commit bc600ae2a17d6338a4e4ee7f500cf0fc47a0ea51) --- CHANGES | 3 +++ doc/notes/notes-current.rst | 3 +++ 2 files changed, 6 insertions(+) diff --git a/CHANGES b/CHANGES index c3abbde0b1..52967a0174 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 ca59158dc6..7c1f7295de 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -44,6 +44,9 @@ Bug Fixes ISC would like to thank Thomas Amgarten for bringing this issue to our attention. :gl:`#4518`, :gl:`#4528` +- Some ISO 8601 durations were accepted erroneously, leading to shorter + durations than expected. This has been fixed. :gl:`#4624` + Known Issues ~~~~~~~~~~~~