[#182] Corrected CVE: CVE-2021-25217

Addressed buffer overwrite in parse_X()

Added Release Note

common/parse.c
    parse_X() - reworked to avoid buffer overwrite on
    over-sized hex literals

common/tests/option_unittest.c
    ATF_TC_BODY(parse_X) - new test which verifies
    parse_X() logic.

3 files changed, 96 insertions, 4 deletions

diff --git a/RELNOTES b/RELNOTES
index 76f75093..e408e656 100644
--- a/RELNOTES
+++ b/RELNOTES
@@ -27,7 +27,7 @@ ISC DHCP is open source software maintained by Internet Systems
 Consortium.  This product includes cryptographic software written
 by Eric Young (eay@cryptsoft.com).
 
-		Changes since 4.4.2 (New Features)
+		Changes since 4.4.2-P1 (New Features)
 
 - BIND libraries updated to the latest version, 9.11.36. This fixes a number
   of compilation issues on various systems, including OpenWRT. Thanks to
@@ -39,7 +39,7 @@ by Eric Young (eay@cryptsoft.com).
   and the client Linux script sample was updated.
   [Gitlab #132]
 
-		Changes since 4.4.2 (Bug Fixes)
+		Changes since 4.4.2-P1 (Bug Fixes)
 
 - Minor corrections to allow compilation under gcc 10.
   [Gitlab #117]
@@ -73,6 +73,13 @@ by Eric Young (eay@cryptsoft.com).
   an object to fail.
   [Gitlab #148]
 
+		Changes since 4.4.2 (Bug Fixes)
+
+- Corrected a buffer overwrite possible when parsing hexadecimal
+  literals with more than 1024 octets.
+  [Gitlab #182]
+  CVE: CVE-2021-25217
+
 		Changes since 4.4.2b1 (Bug Fixes)
 
 - Added a clarification on DHCPINFORMs and server authority to
diff --git a/common/parse.c b/common/parse.c
index 5e3cce7d..b123a6c7 100644
--- a/common/parse.c
+++ b/common/parse.c
@@ -5556,13 +5556,14 @@ int parse_X (cfile, buf, max)
 				skip_to_semi (cfile);
 				return 0;
 			}
-			convert_num (cfile, &buf [len], val, 16, 8);
-			if (len++ > max) {
+			if (len >= max) {
 				parse_warn (cfile,
 					    "hexadecimal constant too long.");
 				skip_to_semi (cfile);
 				return 0;
 			}
+			convert_num (cfile, &buf [len], val, 16, 8);
+			len++;
 			token = peek_token (&val, (unsigned *)0, cfile);
 			if (token == COLON)
 				token = next_token (&val,
diff --git a/common/tests/option_unittest.c b/common/tests/option_unittest.c
index cd52cfb4..bea60837 100644
--- a/common/tests/option_unittest.c
+++ b/common/tests/option_unittest.c
@@ -129,6 +129,89 @@ ATF_TC_BODY(pretty_print_option, tc)
     }
 }
 
+ATF_TC(parse_X);
+
+ATF_TC_HEAD(parse_X, tc)
+{
+    atf_tc_set_md_var(tc, "descr",
+		      "Verify parse_X survices option too big.");
+}
+
+/* Initializes a parse struct from an input buffer of data. */
+static void init_parse(struct parse *cfile, char* name, char *input) {
+    memset(cfile, 0, sizeof(struct parse));
+    cfile->tlname = name;
+    cfile->lpos = cfile->line = 1;
+    cfile->cur_line = cfile->line1;
+    cfile->prev_line = cfile->line2;
+    cfile->token_line = cfile->cur_line;
+    cfile->cur_line[0] = cfile->prev_line[0] = 0;
+    cfile->file = -1;
+    cfile->eol_token = 0;
+
+    cfile->inbuf = input;
+    cfile->buflen = strlen(input);
+    cfile->bufsiz = 0;
+}
+
+/*
+ * This test verifies that parse_X does not overwrite the output
+ * buffer when given input data that exceeds the output buffer
+ * capacity.
+*/
+ATF_TC_BODY(parse_X, tc)
+{
+    struct parse cfile;
+    u_int8_t output[10];
+    unsigned len;
+
+    /* Input hex literal */
+    char *input = "01:02:03:04:05:06:07:08";
+    unsigned expected_len = 8;
+
+    /* Normal output plus two filler bytes */
+    u_int8_t expected_plus_two[] = {
+        0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0xff, 0xff
+    };
+
+    /* Safe output when option is too long */
+    unsigned short_buf_len = 4;
+    u_int8_t expected_too_long[] = {
+        0x01, 0x02, 0x03, 0x04, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff
+    };
+
+    /* First we'll run one that works normally */
+    memset(output, 0xff, sizeof(output));
+    init_parse(&cfile, "hex_fits", input);
+
+    len = parse_X(&cfile, output, expected_len);
+
+    // Len should match the expected len.
+    if (len != expected_len) {
+	    atf_tc_fail("parse_X failed, output len: %d", len);
+    }
+
+    // We should not have written anything past the end of the buffer.
+    if (memcmp(output, expected_plus_two, sizeof(output))) {
+	    atf_tc_fail("parse_X failed, output does not match expected");
+    }
+
+    // Now we'll try it with a buffer that's too small.
+    init_parse(&cfile, "hex_too_long", input);
+    memset(output, 0xff, sizeof(output));
+
+    len = parse_X(&cfile, output, short_buf_len);
+
+    // On errors, len should be zero.
+    if (len != 0) {
+	    atf_tc_fail("parse_X failed, we should have had an error");
+    }
+
+    // We should not have written anything past the end of the buffer.
+    if (memcmp(output, expected_too_long, sizeof(output))) {
+        atf_tc_fail("parse_X overwrote buffer!");
+    }
+}
 
 /* This macro defines main() method that will call specified
    test cases. tp and simple_test_case names can be whatever you want
@@ -137,6 +220,7 @@ ATF_TP_ADD_TCS(tp)
 {
     ATF_TP_ADD_TC(tp, option_refcnt);
     ATF_TP_ADD_TC(tp, pretty_print_option);
+    ATF_TP_ADD_TC(tp, parse_X);
 
     return (atf_no_error());
 }