Merge branch '3429-detect-overflow-in-generate-directive' into 'main'

Detect overflow in $GENERATE directive

Closes #3429

See merge request isc-projects/bind9!6505

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

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}

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

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}

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}

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
+differ from each other by an iterator.
-easily generate the sets of records required to support sub-/24 reverse
+
-delegations described in :rfc:`2317`.
+**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
+This example creates a set of A and MX records. Note the MX's **rdata**
-quoted string. The quotes are stripped when the right-hand side is
+is a quoted string; the quotes are stripped when **$GENERATE** is processed:
-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 example generates A and AAAA records using modifiers; the AAAA
-    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.
+**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.
+::
-
+   HOST-0000.EXAMPLE.   A      1.2.3.1
-**ttl**
+   HOST-0001.EXAMPLE.   A      1.2.3.2
-    This specifies the time-to-live of the generated records. If not specified, this is inherited using the normal TTL inheritance rules.
+   HOST-0002.EXAMPLE.   A      1.2.3.3
-
+   0.0.4.EXAMPLE.       AAAA   2001:db8::400
-    **class** and **ttl** can be entered in either order.
+   1.0.4.EXAMPLE.       AAAA   2001:db8::401
-
+   2.0.4.EXAMPLE.       AAAA   2001:db8::402
-**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.
 
 The **$GENERATE** directive is a BIND extension and not part of the
 standard zone file format.

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,
+				n = sscanf(name,
-					   &width, mode);
+					   "{%d%1[,}]%u%1[,}]%1[doxXnN]%1[}]",
-				switch (n) {
+					   &delta, comma1, &width, comma2, mode,
-				case 1:
+					   brace);
-					break;
+				if (n < 2 || n > 6) {
-				case 2:
+					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;
+				} else if (comma1[0] == ',' &&
-				case 3:
+					   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;
+				} else {
-				default:
 					return (DNS_R_SYNTAX);
 				}
 				if (n >= sizeof(fmt)) {
@@ -725,6 +736,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);