From c5931725b48b121d232df4ba9e45bc41e0ba114d Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Sat, 10 Feb 2018 12:15:27 -0500 Subject: [master] Correct buffer overrun in pretty_print_option Merges in rt47139. --- RELNOTES | 8 +++++- common/options.c | 15 ++++++++-- common/tests/option_unittest.c | 65 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 83 insertions(+), 5 deletions(-) diff --git a/RELNOTES b/RELNOTES index e8d2a5c7..1e203677 100644 --- 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 6f23bc15..fc0e0889 100644 --- 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 (""); + } + 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 36236b84..cd52cfb4 100644 --- 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 \"\""); + } + + + /* 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, "")) { + atf_tc_fail("pretty_print_option did not return \"\""); + } +} + + /* 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()); } -- cgit v1.2.1