Skip to content

Commit 37dcfcc

Browse files
mapleFUpitrou
andauthored
GH-47184: [Parquet][C++] Avoid multiplication overflow in FixedSizeBinaryBuilder::Reserve (#47185)
### Rationale for this change Fix issue found by OSS-Fuzz: https://oss-fuzz.com/testcase?key=6634425377161216 ### What changes are included in this PR? This patch add more checkings directly in builder. ### Are these changes tested? By apache/arrow-testing#110 ### Are there any user-facing changes? no * GitHub Issue: #47184 Lead-authored-by: mwish <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
1 parent 72b0346 commit 37dcfcc

File tree

5 files changed

+23
-4
lines changed

5 files changed

+23
-4
lines changed

cpp/src/arrow/array/builder_binary.cc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,14 @@
3434
#include "arrow/util/bit_util.h"
3535
#include "arrow/util/checked_cast.h"
3636
#include "arrow/util/decimal.h"
37+
#include "arrow/util/int_util_overflow.h"
3738
#include "arrow/util/logging_internal.h"
3839
#include "arrow/visit_data_inline.h"
3940

4041
namespace arrow {
4142

4243
using internal::checked_cast;
44+
using internal::MultiplyWithOverflow;
4345

4446
// ----------------------------------------------------------------------
4547
// Binary/StringView
@@ -162,7 +164,13 @@ void FixedSizeBinaryBuilder::Reset() {
162164

163165
Status FixedSizeBinaryBuilder::Resize(int64_t capacity) {
164166
RETURN_NOT_OK(CheckCapacity(capacity));
165-
RETURN_NOT_OK(byte_builder_.Resize(capacity * byte_width_));
167+
int64_t dest_capacity_bytes;
168+
if (ARROW_PREDICT_FALSE(
169+
MultiplyWithOverflow(capacity, byte_width_, &dest_capacity_bytes))) {
170+
return Status::CapacityError("Resize: capacity overflows (requested: ", capacity,
171+
", byte_width: ", byte_width_, ")");
172+
}
173+
RETURN_NOT_OK(byte_builder_.Resize(dest_capacity_bytes));
166174
return ArrayBuilder::Resize(capacity);
167175
}
168176

cpp/src/arrow/buffer_builder.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ class ARROW_EXPORT BufferBuilder {
6767

6868
/// \brief Resize the buffer to the nearest multiple of 64 bytes
6969
///
70-
/// \param new_capacity the new capacity of the of the builder. Will be
70+
/// \param new_capacity the new capacity of the builder. Will be
7171
/// rounded up to a multiple of 64 bytes for padding
7272
/// \param shrink_to_fit if new capacity is smaller than the existing,
7373
/// reallocate internal buffer. Set to false to avoid reallocations when

cpp/src/parquet/decoder.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2062,6 +2062,17 @@ class DeltaByteArrayDecoderImpl : public TypedDecoderImpl<DType> {
20622062
if (num_valid_values_ == 0) {
20632063
last_value_in_previous_page_ = last_value_;
20642064
}
2065+
2066+
if constexpr (std::is_same_v<DType, FLBAType>) {
2067+
// Checks all values
2068+
for (int i = 0; i < max_values; i++) {
2069+
if (buffer[i].len != static_cast<uint32_t>(this->type_length_)) {
2070+
throw ParquetException("FLBA type requires fixed-length ", this->type_length_,
2071+
" but got ", buffer[i].len);
2072+
}
2073+
}
2074+
}
2075+
20652076
return max_values;
20662077
}
20672078

cpp/submodules/parquet-testing

Submodule parquet-testing updated 287 files

0 commit comments

Comments
 (0)