[master] Correct buffer overrun in pretty_print_option

    Merges in rt47139.

diff --git a/RELNOTES b/RELNOTES
index e8d2a5c740da35f4e3a7eaa84103b56de6ba2ded..1e2036775b74383e51c7afd8ba8b4be3cd9c3d06 100644 (file)
--- a/RELNOTES
+++ b/RELNOTES
@@ -101,7 +101,13 @@ by Eric Young (eay@cryptsoft.com).
   when parsing buffer for options. Reported by Felix Wilhelm, Google
   Security Team.
   [ISC-Bugs #47140]
-  CVE: CVE-2018-xxxx
+  CVE: CVE-2018-5733
+
+! Corrected an issue where large sized 'X/x' format options were causing
+  option handling logic to overwrite memory when expanding them to human
+  readable form. Reported by Felix Wilhelm, Google Security Team.
+  [ISC-Bugs #47139]
+  CVE: CVE-2018-5732
 
                 Changes since 4.4.0b1 (New Features)
 

diff --git a/common/options.c b/common/options.c
index 6f23bc1575229eb06864f76db751a767a2b651a2..fc0e0889939face8afb0a1bf1060d6b432243604 100644 (file)
--- a/common/options.c
+++ b/common/options.c
@@ -1776,7 +1776,8 @@ format_min_length(format, oc)
 
 
 /* Format the specified option so that a human can easily read it. */
-
+/* Maximum pretty printed size */
+#define MAX_OUTPUT_SIZE 32*1024
 const char *pretty_print_option (option, data, len, emit_commas, emit_quotes)
        struct option *option;
        const unsigned char *data;
@@ -1784,8 +1785,9 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes)
        int emit_commas;
        int emit_quotes;
 {
-       static char optbuf [32768]; /* XXX */
-       static char *endbuf = &optbuf[sizeof(optbuf)];
+       /* We add 128 byte pad so we don't have to add checks everywhere. */
+       static char optbuf [MAX_OUTPUT_SIZE + 128]; /* XXX */
+       static char *endbuf = optbuf + MAX_OUTPUT_SIZE;
        int hunksize = 0;
        int opthunk = 0;
        int hunkinc = 0;
@@ -2211,7 +2213,14 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes)
                                log_error ("Unexpected format code %c",
                                           fmtbuf [j]);
                        }
+
                        op += strlen (op);
+                       if (op >= endbuf) {
+                               log_error ("Option data exceeds"
+                                          " maximum size %d", MAX_OUTPUT_SIZE);
+                                          return ("<error>");
+                       }
+
                        if (dp == data + len)
                                break;
                        if (j + 1 < numelem && comma != ':')

diff --git a/common/tests/option_unittest.c b/common/tests/option_unittest.c
index 36236b849cb75773bfcbf4a7208110b5807b75d2..cd52cfb4e2ec07ca46764bc00049fbed0ea1706e 100644 (file)
--- a/common/tests/option_unittest.c
+++ b/common/tests/option_unittest.c
@@ -43,7 +43,7 @@ ATF_TC_BODY(option_refcnt, tc)
     if (!option_state_allocate(&options, MDL)) {
        atf_tc_fail("can't allocate option state");
     }
-    
+
     option = NULL;
     code = 15; /* domain-name */
     if (!option_code_hash_lookup(&option, dhcp_universe.code_hash,
@@ -68,12 +68,75 @@ ATF_TC_BODY(option_refcnt, tc)
     }
 }
 
+ATF_TC(pretty_print_option);
+
+ATF_TC_HEAD(pretty_print_option, tc)
+{
+    atf_tc_set_md_var(tc, "descr",
+                     "Verify pretty_print_option does not overrun its buffer.");
+}
+
+
+/*
+ * This test verifies that pretty_print_option() will not overrun its
+ * internal, static buffer when given large 'x/X' format options.
+ *
+ */
+ATF_TC_BODY(pretty_print_option, tc)
+{
+    struct option *option;
+    unsigned code;
+    unsigned char bad_data[32*1024];
+    unsigned char good_data[] = { 1,2,3,4,5,6 };
+    int emit_commas = 1;
+    int emit_quotes = 1;
+    const char *output_buf;
+
+    /* Initialize whole thing to non-printable chars */
+    memset(bad_data, 0x1f, sizeof(bad_data));
+
+    initialize_common_option_spaces();
+
+    /* We'll use dhcp_client_identitifer because it happens to be format X */
+    code = 61;
+    option = NULL;
+    if (!option_code_hash_lookup(&option, dhcp_universe.code_hash,
+                                &code, 0, MDL)) {
+           atf_tc_fail("can't find option %d", code);
+    }
+
+    if (option == NULL) {
+           atf_tc_fail("option is NULL");
+    }
+
+    /* First we will try a good value we know should fit. */
+    output_buf = pretty_print_option (option, good_data, sizeof(good_data),
+                                      emit_commas, emit_quotes);
+
+    /* Make sure we get what we expect */
+    if (!output_buf || strcmp(output_buf, "1:2:3:4:5:6")) {
+           atf_tc_fail("pretty_print_option did not return \"<error>\"");
+    }
+
+
+    /* Now we'll try a data value that's too large */
+    output_buf = pretty_print_option (option, bad_data, sizeof(bad_data),
+                                      emit_commas, emit_quotes);
+
+    /* Make sure we safely get an error */
+    if (!output_buf || strcmp(output_buf, "<error>")) {
+           atf_tc_fail("pretty_print_option did not return \"<error>\"");
+    }
+}
+
+
 /* This macro defines main() method that will call specified
    test cases. tp and simple_test_case names can be whatever you want
    as long as it is a valid variable identifier. */
 ATF_TP_ADD_TCS(tp)
 {
     ATF_TP_ADD_TC(tp, option_refcnt);
+    ATF_TP_ADD_TC(tp, pretty_print_option);
 
     return (atf_no_error());
 }