[master] Corrected refcnt loss in option parsing

    Merges in 47140.

5 files changed, 121 insertions, 6 deletions

diff --git a/common/options.c b/common/options.c
index 5044d4a1..6f23bc15 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
  *
  * This Source Code Form is subject to the terms of the Mozilla Public
@@ -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/Kyuafile b/common/tests/Kyuafile
index cb1e2cf5..f3c352db 100644
--- a/common/tests/Kyuafile
+++ b/common/tests/Kyuafile
@@ -5,3 +5,4 @@ atf_test_program{name='alloc_unittest'}
 atf_test_program{name='dns_unittest'}
 atf_test_program{name='misc_unittest'}
 atf_test_program{name='ns_name_unittest'}
+atf_test_program{name='option_unittest'}
diff --git a/common/tests/Makefile.am b/common/tests/Makefile.am
index 4129e450..322f77e8 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)
@@ -42,6 +43,14 @@ ns_name_unittest_LDADD += ../libdhcp.@A@ ../../omapip/libomapi.@A@ \
 	@BINDLIBISCCFGDIR@/libisccfg.@A@  \
 	@BINDLIBISCDIR@/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)
 	@if test $(top_srcdir) != ${top_builddir}; then \
 		cp $(top_srcdir)/common/tests/Atffile Atffile; \
diff --git a/common/tests/Makefile.in b/common/tests/Makefile.in
index 60b176f0..89ca98d7 100644
--- a/common/tests/Makefile.in
+++ b/common/tests/Makefile.in
@@ -87,7 +87,9 @@ PRE_UNINSTALL = :
 POST_UNINSTALL = :
 build_triplet = @build@
 host_triplet = @host@
-@HAVE_ATF_TRUE@am__append_1 = alloc_unittest dns_unittest misc_unittest ns_name_unittest
+@HAVE_ATF_TRUE@am__append_1 = alloc_unittest dns_unittest misc_unittest ns_name_unittest \
+@HAVE_ATF_TRUE@	option_unittest
+
 check_PROGRAMS = $(am__EXEEXT_2)
 subdir = common/tests
 ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
@@ -101,7 +103,8 @@ CONFIG_CLEAN_FILES =
 CONFIG_CLEAN_VPATH_FILES =
 @HAVE_ATF_TRUE@am__EXEEXT_1 = alloc_unittest$(EXEEXT) \
 @HAVE_ATF_TRUE@	dns_unittest$(EXEEXT) misc_unittest$(EXEEXT) \
-@HAVE_ATF_TRUE@	ns_name_unittest$(EXEEXT)
+@HAVE_ATF_TRUE@	ns_name_unittest$(EXEEXT) \
+@HAVE_ATF_TRUE@	option_unittest$(EXEEXT)
 am__EXEEXT_2 = $(am__EXEEXT_1)
 am__alloc_unittest_SOURCES_DIST = test_alloc.c \
 	$(top_srcdir)/tests/t_api_dhcp.c
@@ -132,6 +135,13 @@ am__ns_name_unittest_SOURCES_DIST = ns_name_test.c \
 ns_name_unittest_OBJECTS = $(am_ns_name_unittest_OBJECTS)
 @HAVE_ATF_TRUE@ns_name_unittest_DEPENDENCIES = $(am__DEPENDENCIES_1) \
 @HAVE_ATF_TRUE@	../libdhcp.@A@ ../../omapip/libomapi.@A@
+am__option_unittest_SOURCES_DIST = option_unittest.c \
+	$(top_srcdir)/tests/t_api_dhcp.c
+@HAVE_ATF_TRUE@am_option_unittest_OBJECTS = option_unittest.$(OBJEXT) \
+@HAVE_ATF_TRUE@	t_api_dhcp.$(OBJEXT)
+option_unittest_OBJECTS = $(am_option_unittest_OBJECTS)
+@HAVE_ATF_TRUE@option_unittest_DEPENDENCIES = $(am__DEPENDENCIES_1) \
+@HAVE_ATF_TRUE@	../libdhcp.@A@ ../../omapip/libomapi.@A@
 AM_V_P = $(am__v_P_@AM_V@)
 am__v_P_ = $(am__v_P_@AM_DEFAULT_V@)
 am__v_P_0 = false
@@ -165,11 +175,13 @@ am__v_CCLD_ = $(am__v_CCLD_@AM_DEFAULT_V@)
 am__v_CCLD_0 = @echo "  CCLD    " $@;
 am__v_CCLD_1 = 
 SOURCES = $(alloc_unittest_SOURCES) $(dns_unittest_SOURCES) \
-	$(misc_unittest_SOURCES) $(ns_name_unittest_SOURCES)
+	$(misc_unittest_SOURCES) $(ns_name_unittest_SOURCES) \
+	$(option_unittest_SOURCES)
 DIST_SOURCES = $(am__alloc_unittest_SOURCES_DIST) \
 	$(am__dns_unittest_SOURCES_DIST) \
 	$(am__misc_unittest_SOURCES_DIST) \
-	$(am__ns_name_unittest_SOURCES_DIST)
+	$(am__ns_name_unittest_SOURCES_DIST) \
+	$(am__option_unittest_SOURCES_DIST)
 RECURSIVE_TARGETS = all-recursive check-recursive cscopelist-recursive \
 	ctags-recursive dvi-recursive html-recursive info-recursive \
 	install-data-recursive install-dvi-recursive \
@@ -393,6 +405,13 @@ ATF_TESTS = $(am__append_1)
 @HAVE_ATF_TRUE@	@BINDLIBDNSDIR@/libdns.@A@ \
 @HAVE_ATF_TRUE@	@BINDLIBISCCFGDIR@/libisccfg.@A@ \
 @HAVE_ATF_TRUE@	@BINDLIBISCDIR@/libisc.@A@
+@HAVE_ATF_TRUE@option_unittest_SOURCES = option_unittest.c $(top_srcdir)/tests/t_api_dhcp.c
+@HAVE_ATF_TRUE@option_unittest_LDADD = $(ATF_LDFLAGS) ../libdhcp.@A@ \
+@HAVE_ATF_TRUE@	../../omapip/libomapi.@A@ \
+@HAVE_ATF_TRUE@	@BINDLIBIRSDIR@/libirs.@A@ \
+@HAVE_ATF_TRUE@	@BINDLIBDNSDIR@/libdns.@A@ \
+@HAVE_ATF_TRUE@	@BINDLIBISCCFGDIR@/libisccfg.@A@ \
+@HAVE_ATF_TRUE@	@BINDLIBISCDIR@/libisc.@A@
 all: all-recursive
 
 .SUFFIXES:
@@ -446,6 +465,10 @@ ns_name_unittest$(EXEEXT): $(ns_name_unittest_OBJECTS) $(ns_name_unittest_DEPEND
 	@rm -f ns_name_unittest$(EXEEXT)
 	$(AM_V_CCLD)$(LINK) $(ns_name_unittest_OBJECTS) $(ns_name_unittest_LDADD) $(LIBS)
 
+option_unittest$(EXEEXT): $(option_unittest_OBJECTS) $(option_unittest_DEPENDENCIES) $(EXTRA_option_unittest_DEPENDENCIES) 
+	@rm -f option_unittest$(EXEEXT)
+	$(AM_V_CCLD)$(LINK) $(option_unittest_OBJECTS) $(option_unittest_LDADD) $(LIBS)
+
 mostlyclean-compile:
 	-rm -f *.$(OBJEXT)
 
@@ -455,6 +478,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/dns_unittest.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/misc_unittest.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ns_name_test.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/option_unittest.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/t_api_dhcp.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test_alloc.Po@am__quote@
 
diff --git a/common/tests/option_unittest.c b/common/tests/option_unittest.c
new file mode 100644
index 00000000..36236b84
--- /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());
+}