From 197b26f25309f947b97a83b8fdfc414b767798f8 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 9 Feb 2018 14:46:08 -0500 Subject: [master] Corrected refcnt loss in option parsing Merges in 47140. --- common/options.c | 4 ++- common/tests/Kyuafile | 1 + common/tests/Makefile.am | 11 +++++- common/tests/Makefile.in | 32 ++++++++++++++--- common/tests/option_unittest.c | 79 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 121 insertions(+), 6 deletions(-) create mode 100644 common/tests/option_unittest.c (limited to 'common') 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 +#include +#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()); +} -- cgit v1.2.1