From 335e61656fa6034fabc3431a91e5800ba6fc3dc9 Mon Sep 17 00:00:00 2001 From: George Burgess IV Date: Tue, 12 Jul 2022 21:35:52 -0700 Subject: [PATCH] {static_,}range_map: fix overflows under ubsan Explicitly call out where overflows are expected, and add appropriate checking for them. BUG=b:235999011 TEST=Unittests on CrOS and Linux Change-Id: I999a6996183c2f4afc16a1c0188dee3bd64d7f09 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3759630 Reviewed-by: Mike Frysinger --- Makefile.am | 10 +++ Makefile.in | 57 ++++++++++++++- src/common/common.gyp | 2 + src/common/safe_math.h | 82 ++++++++++++++++++++++ src/common/safe_math_unittest.cc | 73 +++++++++++++++++++ src/processor/range_map-inl.h | 12 +++- src/processor/range_map_unittest.cc | 13 ++-- src/processor/static_range_map_unittest.cc | 8 +-- 8 files changed, 242 insertions(+), 15 deletions(-) create mode 100644 src/common/safe_math.h create mode 100644 src/common/safe_math_unittest.cc diff --git a/Makefile.am b/Makefile.am index 2cde8649..514a7ff4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -130,6 +130,7 @@ EXTRA_PROGRAMS = CLEANFILES = check_LIBRARIES += src/testing/libtesting.a +check_PROGRAMS += src/common/safe_math_unittest if !SYSTEM_TEST_LIBS src_testing_libtesting_a_SOURCES = \ @@ -690,6 +691,15 @@ src_tools_mac_dump_syms_dump_syms_mac_CXXFLAGS= \ src_tools_mac_dump_syms_dump_syms_mac_LDADD= \ $(RUSTC_DEMANGLE_LIBS) +src_common_safe_math_unittest_SOURCES = \ + src/common/safe_math.h \ + src/common/safe_math_unittest.cc +src_common_safe_math_unittest_CPPFLAGS = \ + $(AM_CPPFLAGS) $(TEST_CFLAGS) +src_common_safe_math_unittest_LDADD = \ + $(TEST_LIBS) \ + $(PTHREAD_CFLAGS) $(PTHREAD_LIBS) + src_common_dumper_unittest_SOURCES = \ src/common/byte_cursor_unittest.cc \ src/common/convert_UTF.cc \ diff --git a/Makefile.in b/Makefile.in index aaf79133..ab0c822d 100644 --- a/Makefile.in +++ b/Makefile.in @@ -134,7 +134,8 @@ host_triplet = @host@ @LINUX_HOST_TRUE@am__append_3 = -fPIC libexec_PROGRAMS = $(am__EXEEXT_10) bin_PROGRAMS = $(am__EXEEXT_2) $(am__EXEEXT_3) $(am__EXEEXT_4) -check_PROGRAMS = $(am__EXEEXT_5) $(am__EXEEXT_6) $(am__EXEEXT_7) \ +check_PROGRAMS = src/common/safe_math_unittest$(EXEEXT) \ + $(am__EXEEXT_5) $(am__EXEEXT_6) $(am__EXEEXT_7) \ $(am__EXEEXT_8) $(am__EXEEXT_9) EXTRA_PROGRAMS = $(am__EXEEXT_1) @DISABLE_PROCESSOR_FALSE@am__append_4 = src/libbreakpad.a @@ -888,6 +889,15 @@ src_common_mac_macho_reader_unittest_OBJECTS = \ @DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ $(am__DEPENDENCIES_2) \ @DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ $(am__DEPENDENCIES_1) \ @DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ $(am__DEPENDENCIES_1) +am__src_common_safe_math_unittest_SOURCES_DIST = \ + src/common/safe_math.h src/common/safe_math_unittest.cc +@DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@am_src_common_safe_math_unittest_OBJECTS = src/common/safe_math_unittest-safe_math_unittest.$(OBJEXT) +src_common_safe_math_unittest_OBJECTS = \ + $(am_src_common_safe_math_unittest_OBJECTS) +@DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@src_common_safe_math_unittest_DEPENDENCIES = \ +@DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ $(am__DEPENDENCIES_2) \ +@DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ $(am__DEPENDENCIES_1) \ +@DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ $(am__DEPENDENCIES_1) am__src_common_test_assembler_unittest_SOURCES_DIST = \ src/common/test_assembler.cc src/common/test_assembler.h \ src/common/test_assembler_unittest.cc @@ -1701,6 +1711,7 @@ am__depfiles_remade = src/client/$(DEPDIR)/minidump_file_writer.Po \ src/common/$(DEPDIR)/processor_stackwalker_mips_unittest-test_assembler.Po \ src/common/$(DEPDIR)/processor_stackwalker_x86_unittest-test_assembler.Po \ src/common/$(DEPDIR)/processor_synth_minidump_unittest-test_assembler.Po \ + src/common/$(DEPDIR)/safe_math_unittest-safe_math_unittest.Po \ src/common/$(DEPDIR)/string_conversion.Po \ src/common/$(DEPDIR)/test_assembler_unittest-test_assembler.Po \ src/common/$(DEPDIR)/test_assembler_unittest-test_assembler_unittest.Po \ @@ -1970,6 +1981,7 @@ SOURCES = $(src_client_linux_libbreakpad_client_a_SOURCES) \ $(src_common_dwarf_dwarf2reader_splitfunctions_unittest_SOURCES) \ $(src_common_linux_google_crashdump_uploader_test_SOURCES) \ $(src_common_mac_macho_reader_unittest_SOURCES) \ + $(src_common_safe_math_unittest_SOURCES) \ $(src_common_test_assembler_unittest_SOURCES) \ $(src_processor_address_map_unittest_SOURCES) \ $(src_processor_basic_source_line_resolver_unittest_SOURCES) \ @@ -2026,6 +2038,7 @@ DIST_SOURCES = \ $(am__src_common_dwarf_dwarf2reader_splitfunctions_unittest_SOURCES_DIST) \ $(am__src_common_linux_google_crashdump_uploader_test_SOURCES_DIST) \ $(am__src_common_mac_macho_reader_unittest_SOURCES_DIST) \ + $(am__src_common_safe_math_unittest_SOURCES_DIST) \ $(am__src_common_test_assembler_unittest_SOURCES_DIST) \ $(am__src_processor_address_map_unittest_SOURCES_DIST) \ $(am__src_processor_basic_source_line_resolver_unittest_SOURCES_DIST) \ @@ -2909,6 +2922,17 @@ TESTS = $(check_PROGRAMS) $(check_SCRIPTS) @DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@src_tools_mac_dump_syms_dump_syms_mac_LDADD = \ @DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ $(RUSTC_DEMANGLE_LIBS) +@DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@src_common_safe_math_unittest_SOURCES = \ +@DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ src/common/safe_math.h \ +@DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ src/common/safe_math_unittest.cc + +@DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@src_common_safe_math_unittest_CPPFLAGS = \ +@DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ $(AM_CPPFLAGS) $(TEST_CFLAGS) + +@DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@src_common_safe_math_unittest_LDADD = \ +@DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ $(TEST_LIBS) \ +@DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ $(PTHREAD_CFLAGS) $(PTHREAD_LIBS) + @DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@src_common_dumper_unittest_SOURCES = \ @DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ src/common/byte_cursor_unittest.cc \ @DISABLE_TOOLS_FALSE@@LINUX_HOST_TRUE@ src/common/convert_UTF.cc \ @@ -4800,6 +4824,13 @@ src/common/tests/mac_macho_reader_unittest-file_utils.$(OBJEXT): \ src/common/mac/macho_reader_unittest$(EXEEXT): $(src_common_mac_macho_reader_unittest_OBJECTS) $(src_common_mac_macho_reader_unittest_DEPENDENCIES) $(EXTRA_src_common_mac_macho_reader_unittest_DEPENDENCIES) src/common/mac/$(am__dirstamp) @rm -f src/common/mac/macho_reader_unittest$(EXEEXT) $(AM_V_CXXLD)$(CXXLINK) $(src_common_mac_macho_reader_unittest_OBJECTS) $(src_common_mac_macho_reader_unittest_LDADD) $(LIBS) +src/common/safe_math_unittest-safe_math_unittest.$(OBJEXT): \ + src/common/$(am__dirstamp) \ + src/common/$(DEPDIR)/$(am__dirstamp) + +src/common/safe_math_unittest$(EXEEXT): $(src_common_safe_math_unittest_OBJECTS) $(src_common_safe_math_unittest_DEPENDENCIES) $(EXTRA_src_common_safe_math_unittest_DEPENDENCIES) src/common/$(am__dirstamp) + @rm -f src/common/safe_math_unittest$(EXEEXT) + $(AM_V_CXXLD)$(CXXLINK) $(src_common_safe_math_unittest_OBJECTS) $(src_common_safe_math_unittest_LDADD) $(LIBS) src/common/test_assembler_unittest-test_assembler.$(OBJEXT): \ src/common/$(am__dirstamp) \ src/common/$(DEPDIR)/$(am__dirstamp) @@ -5423,6 +5454,7 @@ distclean-compile: @AMDEP_TRUE@@am__include@ @am__quote@src/common/$(DEPDIR)/processor_stackwalker_mips_unittest-test_assembler.Po@am__quote@ # am--include-marker @AMDEP_TRUE@@am__include@ @am__quote@src/common/$(DEPDIR)/processor_stackwalker_x86_unittest-test_assembler.Po@am__quote@ # am--include-marker @AMDEP_TRUE@@am__include@ @am__quote@src/common/$(DEPDIR)/processor_synth_minidump_unittest-test_assembler.Po@am__quote@ # am--include-marker +@AMDEP_TRUE@@am__include@ @am__quote@src/common/$(DEPDIR)/safe_math_unittest-safe_math_unittest.Po@am__quote@ # am--include-marker @AMDEP_TRUE@@am__include@ @am__quote@src/common/$(DEPDIR)/string_conversion.Po@am__quote@ # am--include-marker @AMDEP_TRUE@@am__include@ @am__quote@src/common/$(DEPDIR)/test_assembler_unittest-test_assembler.Po@am__quote@ # am--include-marker @AMDEP_TRUE@@am__include@ @am__quote@src/common/$(DEPDIR)/test_assembler_unittest-test_assembler_unittest.Po@am__quote@ # am--include-marker @@ -7253,6 +7285,20 @@ src/common/tests/mac_macho_reader_unittest-file_utils.obj: src/common/tests/file @AMDEP_TRUE@@am__fastdepCXX_FALSE@ DEPDIR=$(DEPDIR) $(CXXDEPMODE) $(depcomp) @AMDEPBACKSLASH@ @am__fastdepCXX_FALSE@ $(AM_V_CXX@am__nodep@)$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_common_mac_macho_reader_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -c -o src/common/tests/mac_macho_reader_unittest-file_utils.obj `if test -f 'src/common/tests/file_utils.cc'; then $(CYGPATH_W) 'src/common/tests/file_utils.cc'; else $(CYGPATH_W) '$(srcdir)/src/common/tests/file_utils.cc'; fi` +src/common/safe_math_unittest-safe_math_unittest.o: src/common/safe_math_unittest.cc +@am__fastdepCXX_TRUE@ $(AM_V_CXX)$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_common_safe_math_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -MT src/common/safe_math_unittest-safe_math_unittest.o -MD -MP -MF src/common/$(DEPDIR)/safe_math_unittest-safe_math_unittest.Tpo -c -o src/common/safe_math_unittest-safe_math_unittest.o `test -f 'src/common/safe_math_unittest.cc' || echo '$(srcdir)/'`src/common/safe_math_unittest.cc +@am__fastdepCXX_TRUE@ $(AM_V_at)$(am__mv) src/common/$(DEPDIR)/safe_math_unittest-safe_math_unittest.Tpo src/common/$(DEPDIR)/safe_math_unittest-safe_math_unittest.Po +@AMDEP_TRUE@@am__fastdepCXX_FALSE@ $(AM_V_CXX)source='src/common/safe_math_unittest.cc' object='src/common/safe_math_unittest-safe_math_unittest.o' libtool=no @AMDEPBACKSLASH@ +@AMDEP_TRUE@@am__fastdepCXX_FALSE@ DEPDIR=$(DEPDIR) $(CXXDEPMODE) $(depcomp) @AMDEPBACKSLASH@ +@am__fastdepCXX_FALSE@ $(AM_V_CXX@am__nodep@)$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_common_safe_math_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -c -o src/common/safe_math_unittest-safe_math_unittest.o `test -f 'src/common/safe_math_unittest.cc' || echo '$(srcdir)/'`src/common/safe_math_unittest.cc + +src/common/safe_math_unittest-safe_math_unittest.obj: src/common/safe_math_unittest.cc +@am__fastdepCXX_TRUE@ $(AM_V_CXX)$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_common_safe_math_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -MT src/common/safe_math_unittest-safe_math_unittest.obj -MD -MP -MF src/common/$(DEPDIR)/safe_math_unittest-safe_math_unittest.Tpo -c -o src/common/safe_math_unittest-safe_math_unittest.obj `if test -f 'src/common/safe_math_unittest.cc'; then $(CYGPATH_W) 'src/common/safe_math_unittest.cc'; else $(CYGPATH_W) '$(srcdir)/src/common/safe_math_unittest.cc'; fi` +@am__fastdepCXX_TRUE@ $(AM_V_at)$(am__mv) src/common/$(DEPDIR)/safe_math_unittest-safe_math_unittest.Tpo src/common/$(DEPDIR)/safe_math_unittest-safe_math_unittest.Po +@AMDEP_TRUE@@am__fastdepCXX_FALSE@ $(AM_V_CXX)source='src/common/safe_math_unittest.cc' object='src/common/safe_math_unittest-safe_math_unittest.obj' libtool=no @AMDEPBACKSLASH@ +@AMDEP_TRUE@@am__fastdepCXX_FALSE@ DEPDIR=$(DEPDIR) $(CXXDEPMODE) $(depcomp) @AMDEPBACKSLASH@ +@am__fastdepCXX_FALSE@ $(AM_V_CXX@am__nodep@)$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_common_safe_math_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -c -o src/common/safe_math_unittest-safe_math_unittest.obj `if test -f 'src/common/safe_math_unittest.cc'; then $(CYGPATH_W) 'src/common/safe_math_unittest.cc'; else $(CYGPATH_W) '$(srcdir)/src/common/safe_math_unittest.cc'; fi` + src/common/test_assembler_unittest-test_assembler.o: src/common/test_assembler.cc @am__fastdepCXX_TRUE@ $(AM_V_CXX)$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_common_test_assembler_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -MT src/common/test_assembler_unittest-test_assembler.o -MD -MP -MF src/common/$(DEPDIR)/test_assembler_unittest-test_assembler.Tpo -c -o src/common/test_assembler_unittest-test_assembler.o `test -f 'src/common/test_assembler.cc' || echo '$(srcdir)/'`src/common/test_assembler.cc @am__fastdepCXX_TRUE@ $(AM_V_at)$(am__mv) src/common/$(DEPDIR)/test_assembler_unittest-test_assembler.Tpo src/common/$(DEPDIR)/test_assembler_unittest-test_assembler.Po @@ -8846,6 +8892,13 @@ recheck: all $(check_PROGRAMS) $(check_LIBRARIES) $(check_SCRIPTS) am__force_recheck=am--force-recheck \ TEST_LOGS="$$log_list"; \ exit $$? +src/common/safe_math_unittest.log: src/common/safe_math_unittest$(EXEEXT) + @p='src/common/safe_math_unittest$(EXEEXT)'; \ + b='src/common/safe_math_unittest'; \ + $(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \ + --log-file $$b.log --trs-file $$b.trs \ + $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \ + "$$tst" $(AM_TESTS_FD_REDIRECT) src/common/test_assembler_unittest.log: src/common/test_assembler_unittest$(EXEEXT) @p='src/common/test_assembler_unittest$(EXEEXT)'; \ b='src/common/test_assembler_unittest'; \ @@ -9508,6 +9561,7 @@ distclean: distclean-am -rm -f src/common/$(DEPDIR)/processor_stackwalker_mips_unittest-test_assembler.Po -rm -f src/common/$(DEPDIR)/processor_stackwalker_x86_unittest-test_assembler.Po -rm -f src/common/$(DEPDIR)/processor_synth_minidump_unittest-test_assembler.Po + -rm -f src/common/$(DEPDIR)/safe_math_unittest-safe_math_unittest.Po -rm -f src/common/$(DEPDIR)/string_conversion.Po -rm -f src/common/$(DEPDIR)/test_assembler_unittest-test_assembler.Po -rm -f src/common/$(DEPDIR)/test_assembler_unittest-test_assembler_unittest.Po @@ -9853,6 +9907,7 @@ maintainer-clean: maintainer-clean-am -rm -f src/common/$(DEPDIR)/processor_stackwalker_mips_unittest-test_assembler.Po -rm -f src/common/$(DEPDIR)/processor_stackwalker_x86_unittest-test_assembler.Po -rm -f src/common/$(DEPDIR)/processor_synth_minidump_unittest-test_assembler.Po + -rm -f src/common/$(DEPDIR)/safe_math_unittest-safe_math_unittest.Po -rm -f src/common/$(DEPDIR)/string_conversion.Po -rm -f src/common/$(DEPDIR)/test_assembler_unittest-test_assembler.Po -rm -f src/common/$(DEPDIR)/test_assembler_unittest-test_assembler_unittest.Po diff --git a/src/common/common.gyp b/src/common/common.gyp index c1dbb0fe..6fd76952 100644 --- a/src/common/common.gyp +++ b/src/common/common.gyp @@ -163,6 +163,7 @@ 'memory_range.h', 'module.cc', 'module.h', + 'safe_math.h', 'scoped_ptr.h', 'simple_string_dictionary.cc', 'simple_string_dictionary.h', @@ -233,6 +234,7 @@ 'memory_allocator_unittest.cc', 'memory_range_unittest.cc', 'module_unittest.cc', + 'safe_math_unittest.cc', 'simple_string_dictionary_unittest.cc', 'stabs_reader_unittest.cc', 'stabs_to_module_unittest.cc', diff --git a/src/common/safe_math.h b/src/common/safe_math.h new file mode 100644 index 00000000..879e2878 --- /dev/null +++ b/src/common/safe_math.h @@ -0,0 +1,82 @@ +// Copyright (c) 2022, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// safe_math.h: Helpful math functions. +#ifndef SAFE_MATH_H__ +#define SAFE_MATH_H__ + +#include + +namespace google_breakpad { + +// Adds `a` and `b`, returning a pair of: +// - The result after any truncation. +// - Whether an overflow/underflow occurred. +template +std::pair AddWithOverflowCheck(T a, T b) { +#ifdef _WIN32 + // Since C++11, unsigned overflow is well-defined; do everything unsigned, + // assuming 2's complement. + if (std::is_unsigned::value) { + T result = a + b; + // Since we're adding two values >= 0, having a smaller value implies + // overflow. + bool overflow = result < a; + return {result, overflow}; + } + + using TUnsigned = typename std::make_unsigned::type; + T result = TUnsigned(a) + TUnsigned(b); + bool overflow; + if ((a >= 0) == (b >= 0)) { + if (a >= 0) { + overflow = result < a; + } else { + overflow = result > a; + } + } else { + // If signs are different, it's impossible for overflow to happen. + overflow = false; + } + return {result, overflow}; +#else + T result; + bool overflow = __builtin_add_overflow(a, b, &result); + return {result, overflow}; +#endif +} + +template +T AddIgnoringOverflow(T a, T b) { + return AddWithOverflowCheck(a, b).first; +} + +} // namespace google_breakpad + +#endif // SAFE_MATH_H__ diff --git a/src/common/safe_math_unittest.cc b/src/common/safe_math_unittest.cc new file mode 100644 index 00000000..417d96d4 --- /dev/null +++ b/src/common/safe_math_unittest.cc @@ -0,0 +1,73 @@ +// Copyright (c) 2022 Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// safe_math_unittest.cc: Unit tests for SafeMath + +#include "safe_math.h" +#include "breakpad_googletest_includes.h" + +namespace { + +using google_breakpad::AddIgnoringOverflow; +using google_breakpad::AddWithOverflowCheck; + +TEST(SafeMath, AddOverflowWorksAsIntended) { + EXPECT_EQ(AddWithOverflowCheck(0, 0), + std::make_pair(0, false)); + EXPECT_EQ(AddWithOverflowCheck(0, 255), + std::make_pair(255, false)); + EXPECT_EQ(AddWithOverflowCheck(1, 255), + std::make_pair(0, true)); + + EXPECT_EQ(AddWithOverflowCheck(-128, 127), + std::make_pair(-1, false)); + EXPECT_EQ(AddWithOverflowCheck(127, -128), + std::make_pair(-1, false)); + EXPECT_EQ(AddWithOverflowCheck(1, -128), + std::make_pair(-127, false)); + EXPECT_EQ(AddWithOverflowCheck(127, -1), + std::make_pair(126, false)); + + EXPECT_EQ(AddWithOverflowCheck(-128, -1), + std::make_pair(127, true)); + EXPECT_EQ(AddWithOverflowCheck(-128, -128), + std::make_pair(0, true)); + EXPECT_EQ(AddWithOverflowCheck(127, 1), + std::make_pair(-128, true)); + EXPECT_EQ(AddWithOverflowCheck(127, 127), + std::make_pair(-2, true)); +} + +TEST(SafeMath, AddIgnoringOverflowWorksAsIntended) { + EXPECT_EQ(AddIgnoringOverflow(0, 0), 0); + EXPECT_EQ(AddIgnoringOverflow(0, 255), 255); + EXPECT_EQ(AddIgnoringOverflow(1, 255), 0); +} + +} // namespace diff --git a/src/processor/range_map-inl.h b/src/processor/range_map-inl.h index 6bfc7255..ba47893c 100644 --- a/src/processor/range_map-inl.h +++ b/src/processor/range_map-inl.h @@ -39,6 +39,7 @@ #include +#include "common/safe_math.h" #include "processor/range_map.h" #include "processor/linked_ptr.h" #include "processor/logging.h" @@ -57,10 +58,15 @@ template bool RangeMap::StoreRangeInternal( const AddressType& base, const AddressType& delta, const AddressType& size, const EntryType& entry) { - AddressType high = base + (size - 1); - + AddressType high; + bool high_ok = false; + if (size > 0) { + std::pair result = AddWithOverflowCheck(base, size - 1); + high = result.first; + high_ok = !result.second; + } // Check for undersize or overflow. - if (size <= 0 || high < base) { + if (!high_ok) { // The processor will hit this case too frequently with common symbol // files in the size == 0 case, which is more suited to a DEBUG channel. // Filter those out since there's no DEBUG channel at the moment. diff --git a/src/processor/range_map_unittest.cc b/src/processor/range_map_unittest.cc index 0f1c7131..38369bfb 100644 --- a/src/processor/range_map_unittest.cc +++ b/src/processor/range_map_unittest.cc @@ -43,11 +43,10 @@ namespace { - +using google_breakpad::AddIgnoringOverflow; using google_breakpad::linked_ptr; -using google_breakpad::scoped_ptr; using google_breakpad::RangeMap; - +using google_breakpad::scoped_ptr; // A CountedObject holds an int. A global (not thread safe!) count of // allocated CountedObjects is maintained to help test memory management. @@ -148,10 +147,10 @@ static bool RetrieveTest(TestMap* range_map, const RangeTest* range_test) { } for (AddressType offset = low_offset; offset <= high_offset; ++offset) { - AddressType address = - offset + - (!side ? range_test->address : - range_test->address + range_test->size - 1); + AddressType address = AddIgnoringOverflow( + offset, (!side ? range_test->address + : AddIgnoringOverflow(range_test->address, + range_test->size - 1))); bool expected_result = false; // This is correct for tests not stored. if (range_test->expect_storable) { diff --git a/src/processor/static_range_map_unittest.cc b/src/processor/static_range_map_unittest.cc index 1593ed00..22384253 100644 --- a/src/processor/static_range_map_unittest.cc +++ b/src/processor/static_range_map_unittest.cc @@ -228,10 +228,10 @@ void TestStaticRangeMap::RetrieveTest(TestMap* range_map, } for (AddressType offset = low_offset; offset <= high_offset; ++offset) { - AddressType address = - offset + - (!side ? range_test->address : - range_test->address + range_test->size - 1); + AddressType address = AddIgnoringOverflow( + offset, (!side ? range_test->address + : AddIgnoringOverflow(range_test->address, + range_test->size - 1))); bool expected_result = false; // This is correct for tests not stored. if (range_test->expect_storable) {