Handle cases where passing an overly long size could cause UB due to integer overflow

.github/workflows/clang-tidy.yml

@@ -8,14 +8,12 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        image: ["debian:bookworm", "debian:testing", "debian:experimental"]
+        image: ["debian:bookworm", "debian:testing"]
         include:
           - image: "debian:bookworm"
             clang: 15
           - image: "debian:testing"
             clang: 19
-          - image: "debian:experimental"
-            clang: 20
     container:
       image: ${{ matrix.image }}
       env:

CHANGELOG.md

@@ -14,7 +14,15 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 ### Fixed
 
 
-## [1.8.0] - 2024-01-13
+## [1.8.1] - 2025-07-15
+
+### Fixed
+
+- Fix buffer overrun in `get_bool()`
+- Fix test that checks that protozero also works with `std::string_view`
+
+
+## [1.8.0] - 2025-01-13
 
 ### Changed
 

CMakeLists.txt

@@ -10,7 +10,7 @@ cmake_minimum_required(VERSION 3.10.0 FATAL_ERROR)
 
 #-----------------------------------------------------------------------------
 
-project(protozero VERSION 1.8.0 LANGUAGES CXX C)
+project(protozero VERSION 1.8.1 LANGUAGES CXX C)
 
 set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
 

include/protozero/pbf_reader.hpp

@@ -30,6 +30,7 @@ documentation.
 #include <cstddef>
 #include <cstdint>
 #include <cstring>
+#include <limits>
 #include <string>
 #include <utility>
 
@@ -139,6 +140,20 @@ class pbf_reader {
                 T{m_data, m_data}};
     }
 
+    // Adds size to ptr safely, and returning ptr if overflow would occur.
+    const char* safe_ptr_add(const char* ptr, size_t length) {
+#if defined __has_builtin
+#if __has_builtin(__builtin_add_overflow)
+        uintptr_t result;
+        return __builtin_add_overflow(reinterpret_cast<uintptr_t>(ptr), length, &result) ? ptr : reinterpret_cast<const char*>(result);
+#endif
+#endif
+        if (length > std::numeric_limits<uintptr_t>::max() - reinterpret_cast<uintptr_t>(ptr)) {
+            return ptr;
+        }
+        return ptr + length;
+    }
+
 public:
 
     /**
@@ -153,7 +168,7 @@ class pbf_reader {
      */
     explicit pbf_reader(const data_view& view) noexcept
         : m_data{view.data()},
-          m_end{view.data() + view.size()} {
+          m_end{safe_ptr_add(view.data(), view.size())} {
     }
 
     /**
@@ -168,7 +183,7 @@ class pbf_reader {
      */
     pbf_reader(const char* data, std::size_t size) noexcept
         : m_data{data},
-          m_end{data + size} {
+          m_end{safe_ptr_add(data, size)} {
     }
 
 #ifndef PROTOZERO_STRICT_API
@@ -185,7 +200,7 @@ class pbf_reader {
      */
     explicit pbf_reader(const std::pair<const char*, std::size_t>& data) noexcept
         : m_data{data.first},
-          m_end{data.first + data.second} {
+          m_end{safe_ptr_add(data.first, data.second)} {
     }
 #endif
 
@@ -201,7 +216,7 @@ class pbf_reader {
      */
     explicit pbf_reader(const std::string& data) noexcept
         : m_data{data.data()},
-          m_end{data.data() + data.size()} {
+          m_end{safe_ptr_add(data.data(), data.size())} {
     }
 
     /**

include/protozero/varint.hpp

@@ -32,13 +32,15 @@ namespace detail {
 
     // from https://github.com/facebook/folly/blob/master/folly/Varint.h
     inline uint64_t decode_varint_impl(const char** data, const char* end) {
-        const auto* begin = reinterpret_cast<const int8_t*>(*data);
-        const auto* iend = reinterpret_cast<const int8_t*>(end);
-        const int8_t* p = begin;
+        const int8_t* p = reinterpret_cast<const int8_t*>(*data);
+        const int8_t* const iend = reinterpret_cast<const int8_t*>(end);
         uint64_t val = 0;
 
-        if (iend - begin >= max_varint_length) {  // fast path
+        if (iend - p >= max_varint_length) {  // fast path
             do {
+// GCC 12 has trouble understanding that we're not actually taking this branch for short buffers.
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
                 int64_t b = *p++;
                           val  = ((static_cast<uint64_t>(b) & 0x7fU)       ); if (b >= 0) { break; }
                 b = *p++; val |= ((static_cast<uint64_t>(b) & 0x7fU) <<  7U); if (b >= 0) { break; }
@@ -50,6 +52,7 @@ namespace detail {
                 b = *p++; val |= ((static_cast<uint64_t>(b) & 0x7fU) << 49U); if (b >= 0) { break; }
                 b = *p++; val |= ((static_cast<uint64_t>(b) & 0x7fU) << 56U); if (b >= 0) { break; }
                 b = *p++; val |= ((static_cast<uint64_t>(b) & 0x01U) << 63U); if (b >= 0) { break; }
+#pragma GCC diagnostic pop
                 throw varint_too_long_exception{};
             } while (false);
         } else {

include/protozero/version.hpp

@@ -23,12 +23,12 @@ documentation.
 #define PROTOZERO_VERSION_MINOR 8
 
 /// The patch number
-#define PROTOZERO_VERSION_PATCH 0
+#define PROTOZERO_VERSION_PATCH 1
 
 /// The complete version number
 #define PROTOZERO_VERSION_CODE (PROTOZERO_VERSION_MAJOR * 10000 + PROTOZERO_VERSION_MINOR * 100 + PROTOZERO_VERSION_PATCH)
 
 /// Version number as string
-#define PROTOZERO_VERSION_STRING "1.8.0"
+#define PROTOZERO_VERSION_STRING "1.8.1"
 
 #endif // PROTOZERO_VERSION_HPP

test/CMakeLists.txt

@@ -102,7 +102,7 @@ if(PROTOBUF_FOUND)
     target_link_libraries(writer_tests PRIVATE protobuf::libprotobuf-lite)
 
     if(NOT MSVC)
-        set_target_properties(writer_tests PROPERTIES COMPILE_FLAGS "-pthread")
+        set_target_properties(writer_tests PROPERTIES COMPILE_FLAGS "-pthread -Wno-nullability-extension")
         if(NOT APPLE)
             set_target_properties(writer_tests PROPERTIES LINK_FLAGS "-pthread")
         endif()

test/unit/test_basic.cpp

@@ -48,6 +48,20 @@ TEST_CASE("empty buffer in pbf_reader is okay") {
     REQUIRE_FALSE(item.next());
 }
 
+TEST_CASE("buffer that overflows address space") {
+    const char buffer[1] = {0};
+    // Create a pbf_reader with a length that overflows address space.
+    // This should be handled gracefully.
+    // We use a length of size_t(-1) which is the largest possible length.
+    // This should not cause any reads or writes outside of the provided buffer.
+    // The pbf_reader should behave as if the buffer is empty.
+    protozero::pbf_reader item{buffer, size_t(-1)};
+
+    REQUIRE(item.length() == 0);
+    REQUIRE_FALSE(item); // test operator bool()
+    REQUIRE_FALSE(item.next());
+}
+
 TEST_CASE("check every possible value for single byte in buffer") {
     char buffer[1];
     for (int i = 0; i <= 255; ++i) {