Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[security] Fix several vulnerabilities discovered in DSA-4133 #2142

Merged
merged 1 commit into from
Oct 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions src/isc-dhcp/patch/0005-CVE-2017-3144.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
From: Thomas Markwalder <tmark@isc.org>
Date: Thu, 7 Dec 2017 11:23:36 -0500
Subject: [master] Plugs a socket descriptor leak in OMAPI
Origin: https://source.isc.org/cgi-bin/gitweb.cgi?p=dhcp.git;a=commit;h=1a6b62fe17a42b00fa234d06b6dfde3d03451894
Bug: https://bugs.isc.org/Public/Bug/Display.html?id=46767
Bug-Debian: https://bugs.debian.org/887413
Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-3144

Merges in rt46767.
---

diff --git a/omapip/buffer.c b/omapip/buffer.c
index 6e0621b5..a21f0a80 100644
--- a/omapip/buffer.c
+++ b/omapip/buffer.c
@@ -565,6 +565,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 ee15d821..37abbd25 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";
--
2.16.2

145 changes: 145 additions & 0 deletions src/isc-dhcp/patch/0006-CVE-2018-5733.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
From: Zhenggen Xu <zxu@linkedin.com>
Date: Thu, 11 Oct 2018 17:18:32 -0700
Subject: [PATCH] Subject: [master] Corrected refcnt loss in option parsing
Origin:
https://source.isc.org/cgi-bin/gitweb.cgi?p=dhcp.git;a=commit;h=197b26f25309f947b97a83b8fdfc414b767798f8
Bug: https://bugs.isc.org/Public/Bug/Display.html?id=47140 Bug-Debian:
https://bugs.debian.org/891785 Bug-Debian-Security:
https://security-tracker.debian.org/tracker/CVE-2018-5733

Merges in 47140.
---
common/options.c | 2 +
common/tests/Makefile.am | 11 ++++-
common/tests/option_unittest.c | 79 ++++++++++++++++++++++++++++++++++
3 files changed, 91 insertions(+), 1 deletion(-)
create mode 100644 common/tests/option_unittest.c

diff --git a/common/options.c b/common/options.c
index 74f1fb5..db28b5c 100644
--- a/common/options.c
+++ b/common/options.c
@@ -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 "
diff --git a/common/tests/Makefile.am b/common/tests/Makefile.am
index 32e055c..19521c9 100644
--- a/common/tests/Makefile.am
+++ b/common/tests/Makefile.am
@@ -8,7 +8,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)
@@ -34,6 +35,14 @@ ns_name_unittest_LDADD += ../libdhcp.a \
../../omapip/libomapi.a ../../bind/lib/libirs.a \
../../bind/lib/libdns.a ../../bind/lib/libisccfg.a ../../bind/lib/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@ \
+ @BINDLIBIRSDIR@/libirs.@A@ \
+ @BINDLIBDNSDIR@/libdns.@A@ \
+ @BINDLIBISCCFGDIR@/libisccfg.@A@ \
+ @BINDLIBISCDIR@/libisc.@A@
+
check: $(ATF_TESTS)
sh ${top_srcdir}/tests/unittest.sh

diff --git a/common/tests/option_unittest.c b/common/tests/option_unittest.c
new file mode 100644
index 0000000..36236b8
--- /dev/null
+++ b/common/tests/option_unittest.c
@@ -0,0 +1,79 @@
+/*
+ * 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);
+ }
+}
+
+/* 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);
+
+ return (atf_no_error());
+}

144 changes: 144 additions & 0 deletions src/isc-dhcp/patch/0007-CVE-2018-5732.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
From: Thomas Markwalder <tmark@isc.org>
Date: Sat, 10 Feb 2018 12:15:27 -0500
Subject: [master] Correct buffer overrun in pretty_print_option
Origin: https://source.isc.org/cgi-bin/gitweb.cgi?p=dhcp.git;a=commit;h=c5931725b48b121d232df4ba9e45bc41e0ba114d
Bug: https://bugs.isc.org/Public/Bug/Display.html?id=47139
Bug-Debian: https://bugs.debian.org/891786
Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2018-5732

Merges in rt47139.
---

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 ("<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 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 \"<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());
}
--
2.16.2

4 changes: 4 additions & 0 deletions src/isc-dhcp/patch/series
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,7 @@
0002-Support-for-obtaining-name-of-physical-interface-tha.patch
0003-Support-for-loading-port-alias-map-file-to-replace-p.patch
0004-Bugfix-Don-t-print-log-messages-to-stderr-in-release.patch
0005-CVE-2017-3144.patch
0006-CVE-2018-5733.patch
0007-CVE-2018-5732.patch