From 499d4da394b04d052a0a9e13e3caadffb4fd4dc1 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Tue, 15 Jul 2025 17:05:33 +0200 Subject: [PATCH 1/3] Release 1.8.1 --- CHANGELOG.md | 10 +++++++++- CMakeLists.txt | 2 +- include/protozero/version.hpp | 4 ++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b28db15..12231fe4 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/CMakeLists.txt b/CMakeLists.txt index 35cfaa89..b237e397 100644 --- a/CMakeLists.txt +++ b/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) diff --git a/include/protozero/version.hpp b/include/protozero/version.hpp index 18e31cd4..760870f9 100644 --- a/include/protozero/version.hpp +++ b/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 From 65978c0b1f6934302f6f5cafd2b93c7908039d0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Thu, 16 Oct 2025 10:21:31 +0200 Subject: [PATCH 2/3] Handle cases where passing an overly long size could cause UB due to integer overflow --- .github/workflows/clang-tidy.yml | 3 ++- include/protozero/pbf_reader.hpp | 23 +++++++++++++++++++---- include/protozero/varint.hpp | 11 +++++++---- test/CMakeLists.txt | 2 +- test/unit/test_basic.cpp | 14 ++++++++++++++ 5 files changed, 43 insertions(+), 10 deletions(-) diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml index a2c73b72..034893d1 100644 --- a/.github/workflows/clang-tidy.yml +++ b/.github/workflows/clang-tidy.yml @@ -15,7 +15,7 @@ jobs: - image: "debian:testing" clang: 19 - image: "debian:experimental" - clang: 20 + clang: 22 container: image: ${{ matrix.image }} env: @@ -33,6 +33,7 @@ jobs: apt-get install -yq \ clang-${{ matrix.clang }} \ clang-tidy-${{ matrix.clang }} \ + libstdc++-12-dev \ cmake \ libprotobuf-dev \ make \ diff --git a/include/protozero/pbf_reader.hpp b/include/protozero/pbf_reader.hpp index fab4dceb..f8cb86ab 100644 --- a/include/protozero/pbf_reader.hpp +++ b/include/protozero/pbf_reader.hpp @@ -30,6 +30,7 @@ documentation. #include #include #include +#include #include #include @@ -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(ptr), length, &result) ? ptr : reinterpret_cast(result); +#endif +#endif + if (length > std::numeric_limits::max() - reinterpret_cast(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& 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())} { } /** diff --git a/include/protozero/varint.hpp b/include/protozero/varint.hpp index 5c7fcec7..301e9395 100644 --- a/include/protozero/varint.hpp +++ b/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(*data); - const auto* iend = reinterpret_cast(end); - const int8_t* p = begin; + const int8_t* p = reinterpret_cast(*data); + const int8_t* const iend = reinterpret_cast(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(b) & 0x7fU) ); if (b >= 0) { break; } b = *p++; val |= ((static_cast(b) & 0x7fU) << 7U); if (b >= 0) { break; } @@ -50,6 +52,7 @@ namespace detail { b = *p++; val |= ((static_cast(b) & 0x7fU) << 49U); if (b >= 0) { break; } b = *p++; val |= ((static_cast(b) & 0x7fU) << 56U); if (b >= 0) { break; } b = *p++; val |= ((static_cast(b) & 0x01U) << 63U); if (b >= 0) { break; } +#pragma GCC diagnostic pop throw varint_too_long_exception{}; } while (false); } else { diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 3dbd96d1..f9dc0b1d 100644 --- a/test/CMakeLists.txt +++ b/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() diff --git a/test/unit/test_basic.cpp b/test/unit/test_basic.cpp index 14855601..c0a440aa 100644 --- a/test/unit/test_basic.cpp +++ b/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) { From 01feca68bad4649e5609fa1007fed559885c467e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Thu, 16 Oct 2025 11:21:29 +0200 Subject: [PATCH 3/3] remove debian experimental; it's a moving target anyway --- .github/workflows/clang-tidy.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml index 034893d1..7427ad5d 100644 --- a/.github/workflows/clang-tidy.yml +++ b/.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: 22 container: image: ${{ matrix.image }} env: @@ -33,7 +31,6 @@ jobs: apt-get install -yq \ clang-${{ matrix.clang }} \ clang-tidy-${{ matrix.clang }} \ - libstdc++-12-dev \ cmake \ libprotobuf-dev \ make \