diff options
author | Thomas Markwalder <tmark@isc.org> | 2018-02-16 13:51:42 -0500 |
---|---|---|
committer | Thomas Markwalder <tmark@isc.org> | 2018-02-16 13:51:42 -0500 |
commit | 99a25aedea02d9c259cb8fabf4be700fb32571a3 (patch) | |
tree | 7909b6bbf42b549a4528be2b7a02400bcc67d91f | |
parent | c1bd79a15e9b689e8a18538aaedec0ab8bd04b17 (diff) | |
download | isc-dhcp-99a25aedea02d9c259cb8fabf4be700fb32571a3.tar.gz |
[v4_3_6_p1] Added fixes for CVE-2018-5732, CVE-2018-5733, and #46767
Added patches and unit tests
-rw-r--r-- | .gitignore | 1 | ||||
-rw-r--r-- | RELNOTES | 23 | ||||
-rw-r--r-- | common/options.c | 18 | ||||
-rw-r--r-- | common/tests/Makefile.am | 9 | ||||
-rw-r--r-- | common/tests/option_unittest.c | 142 | ||||
-rw-r--r-- | omapip/buffer.c | 9 | ||||
-rw-r--r-- | omapip/message.c | 2 |
7 files changed, 196 insertions, 8 deletions
@@ -11,6 +11,7 @@ common/tests/alloc_unittest common/tests/dns_unittest common/tests/misc_unittest common/tests/ns_name_unittest +common/tests/option_unittest config.log config.report config.status @@ -1,6 +1,6 @@ Internet Systems Consortium DHCP Distribution - Version 4.3.6 - 31 July 2017 + Version 4.3.6-P1 + 28 February 2018 Release Notes @@ -66,6 +66,25 @@ We welcome comments from DHCP users, about this or anything else we do. Email Vicky Risk, Product Manager at vicky@isc.org or discuss on dhcp-users@lists.isc.org. + Changes since 4.3.6 + +!- Plugged a socket descriptor leak in OMAPI, that can occur when there is + data pending to be written to an OMAPI connection, when the connection + is closed by the reader. + [ISc-Bugs #46767] + +! 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 + +! Option reference count was not correctly decremented in error path + when parsing buffer for options. Reported by Felix Wilhelm, Google + Security Team. + [ISC-Bugs #47140] + CVE: CVE-2018-xxxx + Changes since 4.3.6b1 - None diff --git a/common/options.c b/common/options.c index 5547287f..09cd7857 100644 --- a/common/options.c +++ b/common/options.c @@ -3,7 +3,7 @@ DHCP options parsing and reassembly. */ /* - * Copyright (c) 2004-2017 by Internet Systems Consortium, Inc. ("ISC") + * Copyright (c) 2004-2018 by Internet Systems Consortium, Inc. ("ISC") * Copyright (c) 1995-2003 by Internet Software Consortium * * Permission to use, copy, modify, and distribute this software for any @@ -177,6 +177,8 @@ int parse_option_buffer (options, buffer, length, universe) /* If the length is outrageous, the options are bad. */ if (offset + len > length) { + /* Avoid reference count overflow */ + option_dereference(&option, MDL); reason = "option length exceeds option buffer length"; bogus: log_error("parse_option_buffer: malformed option " @@ -1758,7 +1760,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; @@ -1766,8 +1769,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; @@ -2194,6 +2198,12 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes) 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/Makefile.am b/common/tests/Makefile.am index f6a43e42..8254a1b7 100644 --- a/common/tests/Makefile.am +++ b/common/tests/Makefile.am @@ -10,7 +10,8 @@ ATF_TESTS = if HAVE_ATF -ATF_TESTS += alloc_unittest dns_unittest misc_unittest ns_name_unittest +ATF_TESTS += alloc_unittest dns_unittest misc_unittest ns_name_unittest \ + option_unittest alloc_unittest_SOURCES = test_alloc.c $(top_srcdir)/tests/t_api_dhcp.c alloc_unittest_LDADD = $(ATF_LDFLAGS) @@ -36,6 +37,12 @@ ns_name_unittest_LDADD += ../libdhcp.a \ ../../omapip/libomapi.a $(BINDLIBDIR)/libirs.a \ $(BINDLIBDIR)/libdns.a $(BINDLIBDIR)/libisccfg.a $(BINDLIBDIR)/libisc.a +option_unittest_SOURCES = option_unittest.c $(top_srcdir)/tests/t_api_dhcp.c +option_unittest_LDADD = $(ATF_LDFLAGS) +option_unittest_LDADD += ../libdhcp.a \ + ../../omapip/libomapi.a $(BINDLIBDIR)/libirs.a \ + $(BINDLIBDIR)/libdns.a $(BINDLIBDIR)/libisccfg.a $(BINDLIBDIR)/libisc.a + check: $(ATF_TESTS) @if test $(top_srcdir) != ${top_builddir}; then \ cp $(top_srcdir)/common/tests/Atffile Atffile; \ diff --git a/common/tests/option_unittest.c b/common/tests/option_unittest.c new file mode 100644 index 00000000..cd52cfb4 --- /dev/null +++ b/common/tests/option_unittest.c @@ -0,0 +1,142 @@ +/* + * Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") + * + * 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 http://mozilla.org/MPL/2.0/. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH + * REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY + * AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, + * INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM + * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE + * OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR + * PERFORMANCE OF THIS SOFTWARE. + */ + +#include <config.h> +#include <atf-c.h> +#include "dhcpd.h" + +ATF_TC(option_refcnt); + +ATF_TC_HEAD(option_refcnt, tc) +{ + atf_tc_set_md_var(tc, "descr", + "Verify option reference count does not overflow."); +} + +/* This test does a simple check to see if option reference count is + * decremented even an error path exiting parse_option_buffer() + */ +ATF_TC_BODY(option_refcnt, tc) +{ + struct option_state *options; + struct option *option; + unsigned code; + int refcnt; + unsigned char buffer[3] = { 15, 255, 0 }; + + initialize_common_option_spaces(); + + options = NULL; + 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, + &code, 0, MDL)) { + atf_tc_fail("can't find option 15"); + } + if (option == NULL) { + atf_tc_fail("option is NULL"); + } + refcnt = option->refcnt; + + buffer[0] = 15; + buffer[1] = 255; /* invalid */ + buffer[2] = 0; + + if (parse_option_buffer(options, buffer, 3, &dhcp_universe)) { + atf_tc_fail("parse_option_buffer is expected to fail"); + } + + if (refcnt != option->refcnt) { + atf_tc_fail("refcnt changed from %d to %d", refcnt, option->refcnt); + } +} + +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()); +} diff --git a/omapip/buffer.c b/omapip/buffer.c index f7fdc325..809034d1 100644 --- a/omapip/buffer.c +++ b/omapip/buffer.c @@ -566,6 +566,15 @@ isc_result_t omapi_connection_writer (omapi_object_t *h) omapi_buffer_dereference (&buffer, MDL); } } + + /* If we had data left to write when we're told to disconnect, + * we need recall disconnect, now that we're done writing. + * See rt46767. */ + if (c->out_bytes == 0 && c->state == omapi_connection_disconnecting) { + omapi_disconnect (h, 1); + return ISC_R_SHUTTINGDOWN; + } + return ISC_R_SUCCESS; } diff --git a/omapip/message.c b/omapip/message.c index 59ccdc2c..21bcfc38 100644 --- a/omapip/message.c +++ b/omapip/message.c @@ -339,7 +339,7 @@ isc_result_t omapi_message_unregister (omapi_object_t *mo) } #ifdef DEBUG_PROTOCOL -static const char *omapi_message_op_name(int op) { +const char *omapi_message_op_name(int op) { switch (op) { case OMAPI_OP_OPEN: return "OMAPI_OP_OPEN"; case OMAPI_OP_REFRESH: return "OMAPI_OP_REFRESH"; |