From 5327b9708fd0e5d0d6c95183cca9eafb4a1cfe05 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 1 Jul 2022 11:40:37 +1000 Subject: [PATCH 1/4] Check for overflow in $GENERATE computations $GENERATE uses 'int' for its computations and some constructions can overflow values that can be represented by an 'int' resulting in undefined behaviour. Detect these conditions and return a range error. --- .../checkzone/zones/bad-generate-range.db | 18 ++++++++++++++++++ lib/dns/master.c | 7 +++++++ 2 files changed, 25 insertions(+) create mode 100644 bin/tests/system/checkzone/zones/bad-generate-range.db diff --git a/bin/tests/system/checkzone/zones/bad-generate-range.db b/bin/tests/system/checkzone/zones/bad-generate-range.db new file mode 100644 index 0000000000..62a9e15684 --- /dev/null +++ b/bin/tests/system/checkzone/zones/bad-generate-range.db @@ -0,0 +1,18 @@ +; 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. + +$TTL 600 +@ SOA ns hostmaster 2011012708 3600 1200 604800 1200 + NS ns +ns A 192.0.2.1 + +; 2147483647 + 1 overflows what can be represented in an 'int' +$GENERATE 1-1 host$ TXT foo${2147483647} diff --git a/lib/dns/master.c b/lib/dns/master.c index 598bde02de..0933d50e4c 100644 --- a/lib/dns/master.c +++ b/lib/dns/master.c @@ -725,6 +725,13 @@ genname(char *name, int it, char *buffer, size_t length) { continue; } } + /* + * 'it' is >= 0 so we don't need to check for + * underflow. + */ + if ((it > 0 && delta > INT_MAX - it)) { + return (ISC_R_RANGE); + } if (nibblemode) { n = nibbles(numbuf, sizeof(numbuf), width, mode[0], it + delta); From 7be64c0e94c967c0014a0b960a495c4fb05f1fc2 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 1 Jul 2022 11:13:51 +1000 Subject: [PATCH 2/4] Tighten $GENERATE directive parsing The original sscanf processing allowed for a number of syntax errors to be accepted. This included missing the closing brace in ${modifiers} Look for both comma and right brace as intermediate seperators as well as consuming the final right brace in the sscanf processing for ${modifiers}. Check when we got right brace to determine if the sscanf consumed more input than expected and if so behave as if it had stopped at the first right brace. --- .../checkzone/zones/bad-generate-garbage.db | 17 ++++++++++ .../zones/bad-generate-missing-brace.db | 17 ++++++++++ .../checkzone/zones/good-generate-modifier.db | 20 +++++++++++ lib/dns/master.c | 33 ++++++++++++------- 4 files changed, 76 insertions(+), 11 deletions(-) create mode 100644 bin/tests/system/checkzone/zones/bad-generate-garbage.db create mode 100644 bin/tests/system/checkzone/zones/bad-generate-missing-brace.db create mode 100644 bin/tests/system/checkzone/zones/good-generate-modifier.db diff --git a/bin/tests/system/checkzone/zones/bad-generate-garbage.db b/bin/tests/system/checkzone/zones/bad-generate-garbage.db new file mode 100644 index 0000000000..0d66e753b6 --- /dev/null +++ b/bin/tests/system/checkzone/zones/bad-generate-garbage.db @@ -0,0 +1,17 @@ +; 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. + +$TTL 600 +@ SOA ns hostmaster 2011012708 3600 1200 604800 1200 + NS ns +ns A 192.0.2.1 + +$GENERATE 0-7 host$ A 1.2.3.${1,0,dgarbagegarbage} diff --git a/bin/tests/system/checkzone/zones/bad-generate-missing-brace.db b/bin/tests/system/checkzone/zones/bad-generate-missing-brace.db new file mode 100644 index 0000000000..314583e71a --- /dev/null +++ b/bin/tests/system/checkzone/zones/bad-generate-missing-brace.db @@ -0,0 +1,17 @@ +; 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. + +$TTL 600 +@ SOA ns hostmaster 2011012708 3600 1200 604800 1200 + NS ns +ns A 192.0.2.1 + +$GENERATE 0-7 host$ A 1.2.3.${1000 diff --git a/bin/tests/system/checkzone/zones/good-generate-modifier.db b/bin/tests/system/checkzone/zones/good-generate-modifier.db new file mode 100644 index 0000000000..3c811d60e0 --- /dev/null +++ b/bin/tests/system/checkzone/zones/good-generate-modifier.db @@ -0,0 +1,20 @@ +; 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. + +$TTL 600 +@ SOA ns hostmaster 2011012708 3600 1200 604800 1200 + NS ns +ns A 192.0.2.1 + +$GENERATE 0-7 host$ A 1.2.3.${1,0,d} +$GENERATE 8-9 host$ A 1.2.3.${1,0} +$GENERATE 10-11 host$ A 1.2.3.${1} +$GENERATE 1024-1026 ${0,3,n} AAAA 2001:db8::${0,4,x} diff --git a/lib/dns/master.c b/lib/dns/master.c index 0933d50e4c..f733f4cfca 100644 --- a/lib/dns/master.c +++ b/lib/dns/master.c @@ -673,7 +673,10 @@ genname(char *name, int it, char *buffer, size_t length) { char fmt[sizeof("%04000000000d")]; char numbuf[128]; char *cp; - char mode[2]; + char mode[2] = { 0 }; + char brace[2] = { 0 }; + char comma1[2] = { 0 }; + char comma2[2] = { 0 }; int delta = 0; isc_textregion_t r; unsigned int n; @@ -698,23 +701,31 @@ genname(char *name, int it, char *buffer, size_t length) { strlcpy(fmt, "%d", sizeof(fmt)); /* Get format specifier. */ if (*name == '{') { - n = sscanf(name, "{%d,%u,%1[doxXnN]}", &delta, - &width, mode); - switch (n) { - case 1: - break; - case 2: + n = sscanf(name, + "{%d%1[,}]%u%1[,}]%1[doxXnN]%1[}]", + &delta, comma1, &width, comma2, mode, + brace); + if (n < 2 || n > 6) { + return (DNS_R_SYNTAX); + } + if (comma1[0] == '}') { + /* %{delta} */ + } else if (comma1[0] == ',' && comma2[0] == '}') + { + /* %{delta,width} */ n = snprintf(fmt, sizeof(fmt), "%%0%ud", width); - break; - case 3: + } else if (comma1[0] == ',' && + comma2[0] == ',' && mode[0] != 0 && + brace[0] == '}') + { + /* %{delta,width,format} */ if (mode[0] == 'n' || mode[0] == 'N') { nibblemode = true; } n = snprintf(fmt, sizeof(fmt), "%%0%u%c", width, mode[0]); - break; - default: + } else { return (DNS_R_SYNTAX); } if (n >= sizeof(fmt)) { From 13fb2faf7a875198e86fa134e42bb150e14ec53f Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 1 Jul 2022 21:38:11 -0700 Subject: [PATCH 3/4] Improve $GENERATE documentation Clarify the documentation of $GENERATE modifiers and add an example. --- doc/arm/zones.inc.rst | 115 ++++++++++++++++++++++++++++++------------ 1 file changed, 84 insertions(+), 31 deletions(-) diff --git a/doc/arm/zones.inc.rst b/doc/arm/zones.inc.rst index eb7cc4090e..60289fe272 100644 --- a/doc/arm/zones.inc.rst +++ b/doc/arm/zones.inc.rst @@ -336,12 +336,76 @@ TTLs. Valid TTLs are of the range 0-2147483647 seconds. BIND Primary File Extension: the **$GENERATE** Directive ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Syntax: **$GENERATE** range lhs [ttl] [class] type rhs [comment] +Syntax: **$GENERATE** range owner [ttl] [class] type rdata [comment] **$GENERATE** is used to create a series of resource records that only -differ from each other by an iterator. **$GENERATE** can be used to -easily generate the sets of records required to support sub-/24 reverse -delegations described in :rfc:`2317`. +differ from each other by an iterator. + +**range** + This can be one of two forms: start-stop or start-stop/step. + If the first form is used, then step is set to 1. "start", + "stop", and "step" must be positive integers between 0 and + (2^31)-1. "start" must not be larger than "stop". + +**owner** + This describes the owner name of the resource records to be created. + + The **owner** string may include one or more **$** (dollar sign) + symbols, which will be replaced with the iterator value when + generating records; see below for details. + +**ttl** + This specifies the time-to-live of the generated records. If + not specified, this is inherited using the normal TTL inheritance + rules. + + **class** and **ttl** can be entered in either order. + +**class** + This specifies the class of the generated records. This must + match the zone class if it is specified. + + **class** and **ttl** can be entered in either order. + +**type** + This can be any valid type. + +**rdata** + This is a string containing the RDATA of the resource record + to be created. As with **owner**, the **rdata** string may + include one or more **$** symbols, which are replaced with the + iterator value. **rdata** may be quoted if there are spaces in + the string; the quotation marks do not appear in the generated + record. + + Any single **$** (dollar sign) symbols within the **owner** or + **rdata** strings are replaced by the iterator value. To get a **$** + in the output, escape the **$** using a backslash **\\**, e.g., + ``\$``. (For compatibility with earlier versions, **$$** is also + recognized as indicating a literal **$** in the output.) + + The **$** may optionally be followed by modifiers which change + the offset from the iterator, field width, and base. Modifiers + are introduced by a **{** (left brace) immediately following + the **$**, as in **${offset[,width[,base]]}**. For example, + **${-20,3,d}** subtracts 20 from the current value and prints + the result as a decimal in a zero-padded field of width 3. + Available output forms are decimal (**d**), octal (**o**), + hexadecimal (**x** or **X** for uppercase), and nibble (**n** + or **N** for uppercase). The modfiier cannot contain whitespace + or newlines. + + The default modifier is **${0,0,d}**. If the **owner** is not + absolute, the current **$ORIGIN** is appended to the name. + + In nibble mode, the value is treated as if it were a reversed + hexadecimal string, with each hexadecimal digit as a separate + label. The width field includes the label separator. + +Examples: + +**$GENERATE** can be used to easily generate the sets of records required +to support sub-/24 reverse delegations described in :rfc:`2317`: :: @@ -360,9 +424,8 @@ is equivalent to ... 127.0.0.192.IN-ADDR.ARPA. CNAME 127.0.0.0.192.IN-ADDR.ARPA. -Both generate a set of A and MX records. Note the MX's right-hand side is a -quoted string. The quotes are stripped when the right-hand side is -processed. +This example creates a set of A and MX records. Note the MX's **rdata** +is a quoted string; the quotes are stripped when **$GENERATE** is processed: :: @@ -384,35 +447,25 @@ is equivalent to HOST-127.EXAMPLE. A 1.2.3.127 HOST-127.EXAMPLE. MX 0 . -**range** - This can be one of two forms: start-stop or start-stop/step. If the first form is used, then step is set to 1. "start", "stop", and "step" must be positive integers between 0 and (2^31)-1. "start" must not be larger than "stop". -**owner** - This describes the owner name of the resource records to be created. Any single **$** (dollar sign) symbols within the **owner** string are replaced by the iterator value. To get a **$** in the output, escape the **$** using a backslash **\\**, e.g., ``\$``. The **$** may optionally be followed by modifiers which change the offset from the iterator, field width, and base. +This example generates A and AAAA records using modifiers; the AAAA +**owner** names are generated using nibble mode: - Modifiers are introduced by a **{** (left brace) immediately following the **$**, as in **${offset[,width[,base]]}**. For example, **${-20,3,d}** subtracts 20 from the current value and prints the result as a decimal in a zero-padded field of width 3. Available output forms are decimal (**d**), octal (**o**), hexadecimal (**x** or **X** for uppercase), and nibble (**n** or **N** for uppercase). +:: - The default modifier is **${0,0,d}**. If the **owner** is not absolute, the current **$ORIGIN** is appended to the name. + $ORIGIN EXAMPLE. + $GENERATE 0-2 HOST-${0,4,d} A 1.2.3.${1,0,d} + $GENERATE 1024-1026 ${0,3,n} AAAA 2001:db8::${0,4,x} - In nibble mode, the value is treated as if it were a reversed hexadecimal string, with each hexadecimal digit as a separate label. The width field includes the label separator. +is equivalent to: - For compatibility with earlier versions, **$$** is still recognized as indicating a literal **$** in the output. - -**ttl** - This specifies the time-to-live of the generated records. If not specified, this is inherited using the normal TTL inheritance rules. - - **class** and **ttl** can be entered in either order. - -**class** - This specifies the class of the generated records. This must match the zone class if it is specified. - - **class** and **ttl** can be entered in either order. - -**type** - This can be any valid type. - -**rdata** - This is a string containing the RDATA of the resource record to be created. It may be quoted if there are spaces in the string; the quotation marks do not appear in the generated record. +:: + HOST-0000.EXAMPLE. A 1.2.3.1 + HOST-0001.EXAMPLE. A 1.2.3.2 + HOST-0002.EXAMPLE. A 1.2.3.3 + 0.0.4.EXAMPLE. AAAA 2001:db8::400 + 1.0.4.EXAMPLE. AAAA 2001:db8::401 + 2.0.4.EXAMPLE. AAAA 2001:db8::402 The **$GENERATE** directive is a BIND extension and not part of the standard zone file format. From d935ead14b47200f2a572e4c7011812b0debbd6d Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 1 Jul 2022 11:50:23 +1000 Subject: [PATCH 4/4] Add CHANGES note for [GL #3429] --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index 8d8a465a01..6608a674cd 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +5915. [bug] Detect missing closing brace (}) and computational + overflows in $GENERATE directives. [GL #3429] + 5914. [bug] When synth-from-dnssec generated a response using records from a higher zone, it could unexpectedly prove non-existance of records in a subordinate grafted-on