From a4bbb84d43b564508997d6207f7ec183349b8e9e Mon Sep 17 00:00:00 2001 From: jmestwa-coder Date: Mon, 25 May 2026 21:22:47 +0530 Subject: [PATCH 1/2] avoid 32-bit size overflow in WKBBuffer::ReadCoords bounds check --- cpp/src/parquet/geospatial/util_internal.cc | 2 +- .../parquet/geospatial/util_internal_test.cc | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/cpp/src/parquet/geospatial/util_internal.cc b/cpp/src/parquet/geospatial/util_internal.cc index 59551ae87035..25a709306af5 100644 --- a/cpp/src/parquet/geospatial/util_internal.cc +++ b/cpp/src/parquet/geospatial/util_internal.cc @@ -64,7 +64,7 @@ class WKBBuffer { template void ReadCoords(uint32_t n_coords, bool swap, Visit&& visit) { - size_t total_bytes = n_coords * sizeof(Coord); + uint64_t total_bytes = static_cast(n_coords) * sizeof(Coord); if (size_ < total_bytes) { throw ParquetException("Can't read coordinate sequence of ", total_bytes, " bytes from WKBBuffer with ", size_, " remaining"); diff --git a/cpp/src/parquet/geospatial/util_internal_test.cc b/cpp/src/parquet/geospatial/util_internal_test.cc index 527bcc505051..c12d9d53c237 100644 --- a/cpp/src/parquet/geospatial/util_internal_test.cc +++ b/cpp/src/parquet/geospatial/util_internal_test.cc @@ -166,6 +166,23 @@ TEST_P(WKBTestFixture, TestWKBBounderErrorForInputWithTooManyBytes) { ASSERT_THROW(bounder.MergeGeometry(wkb_with_extra_byte), ParquetException); } +TEST(TestGeometryUtil, TestWKBBounderErrorForCoordinateSequenceSizeOverflow) { + WKBGeometryBounder bounder; + + // LINESTRING with 0x10000001 XY coordinates but only one coordinate in the buffer. + // On 32-bit targets, 0x10000001 * sizeof(XY) would overflow to 16 bytes. + std::vector wkb = { + 0x01, // little endian + 0x02, 0x00, 0x00, 0x00, // LINESTRING + 0x01, 0x00, 0x00, 0x10, // 0x10000001 coordinates + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // x = 0 + 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // y = 0 + 0x00}; + + ASSERT_THROW(bounder.MergeGeometry(wkb), ParquetException); +} + INSTANTIATE_TEST_SUITE_P( TestGeometryUtil, WKBTestFixture, ::testing::Values( From bc59729ec27d602ff91c9e796ce0e26f68479167 Mon Sep 17 00:00:00 2001 From: jmestwa-coder Date: Thu, 28 May 2026 12:40:43 +0530 Subject: [PATCH 2/2] use MultiplyWithOverflow for coordinate sequence size --- cpp/src/parquet/geospatial/util_internal.cc | 8 ++++++-- cpp/src/parquet/geospatial/util_internal_test.cc | 14 ++++++-------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/cpp/src/parquet/geospatial/util_internal.cc b/cpp/src/parquet/geospatial/util_internal.cc index 25a709306af5..54e82f6c0458 100644 --- a/cpp/src/parquet/geospatial/util_internal.cc +++ b/cpp/src/parquet/geospatial/util_internal.cc @@ -21,6 +21,7 @@ #include #include "arrow/util/endian.h" +#include "arrow/util/int_util_overflow.h" #include "arrow/util/macros.h" #include "arrow/util/ubsan.h" #include "parquet/exception.h" @@ -64,8 +65,11 @@ class WKBBuffer { template void ReadCoords(uint32_t n_coords, bool swap, Visit&& visit) { - uint64_t total_bytes = static_cast(n_coords) * sizeof(Coord); - if (size_ < total_bytes) { + uint64_t total_bytes = 0; + if (::arrow::internal::MultiplyWithOverflow(static_cast(n_coords), + static_cast(sizeof(Coord)), + &total_bytes) || + size_ < total_bytes) { throw ParquetException("Can't read coordinate sequence of ", total_bytes, " bytes from WKBBuffer with ", size_, " remaining"); } diff --git a/cpp/src/parquet/geospatial/util_internal_test.cc b/cpp/src/parquet/geospatial/util_internal_test.cc index c12d9d53c237..4104fe6afb91 100644 --- a/cpp/src/parquet/geospatial/util_internal_test.cc +++ b/cpp/src/parquet/geospatial/util_internal_test.cc @@ -171,14 +171,12 @@ TEST(TestGeometryUtil, TestWKBBounderErrorForCoordinateSequenceSizeOverflow) { // LINESTRING with 0x10000001 XY coordinates but only one coordinate in the buffer. // On 32-bit targets, 0x10000001 * sizeof(XY) would overflow to 16 bytes. - std::vector wkb = { - 0x01, // little endian - 0x02, 0x00, 0x00, 0x00, // LINESTRING - 0x01, 0x00, 0x00, 0x10, // 0x10000001 coordinates - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // x = 0 - 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // y = 0 - 0x00}; + std::vector wkb = {0x01, // little endian + 0x02, 0x00, 0x00, 0x00, // LINESTRING + 0x01, 0x00, 0x00, 0x10, // 0x10000001 coordinates + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // x = 0 + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // y = 0 + 0x00}; ASSERT_THROW(bounder.MergeGeometry(wkb), ParquetException); }