From f4c2f2b94bba60cfffe727e9c6a63a2c9d2cc936 Mon Sep 17 00:00:00 2001 From: Mosha Pasumansky Date: Wed, 16 Sep 2026 08:18:19 -0700 Subject: [PATCH] fix(orc): reject out-of-range DECIMAL precision/scale instead of aborting (FB-4175) The ORC adapter's GetArrowType() reads a DECIMAL type's precision and scale verbatim from the (untrusted) ORC footer and forwards them to decimal128(). decimal128() constructs a Decimal128Type, whose constructor enforces 1 <= precision <= 38 with ARROW_CHECK_OK(ValidateDecimalPrecision<...>) -- which logs FATAL and aborts the process rather than returning a Status. A malformed footer declaring e.g. precision 48 therefore crashes any caller of ORCFileReader::ReadSchema() on otherwise well-formed input, a denial of service on untrusted ORC. Scale is not validated at all. Validate precision and scale in the DECIMAL case and return Status::TypeError on violation, keeping the precision == 0 legacy HIVE sentinel. The raw uint64 footer values are compared before the narrowing static_cast, so a precision above INT_MAX cannot wrap into the valid range. GetArrowType() already returns Result<>, so the error propagates to ReadSchema() instead of aborting. Add a self-contained adapter_test.cc unit test driving GetArrowType() over createDecimalType() for precision > 38, precision > INT_MAX, scale > precision, the HIVE precision == 0 sentinel, and valid decimals including scale == precision. Co-Authored-By: Claude Opus 4.8 --- cpp/src/arrow/adapters/orc/adapter_test.cc | 31 ++++++++++++++++++++++ cpp/src/arrow/adapters/orc/util.cc | 16 ++++++++--- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/adapters/orc/adapter_test.cc b/cpp/src/arrow/adapters/orc/adapter_test.cc index b671f26a8df5..69f913c6d5f5 100644 --- a/cpp/src/arrow/adapters/orc/adapter_test.cc +++ b/cpp/src/arrow/adapters/orc/adapter_test.cc @@ -658,6 +658,37 @@ TEST(TestAdapterReadWrite, ThrowWhenTZDBUnavaiable) { Raises(StatusCode::Invalid, testing::HasSubstr(expect_str))); } +// An out-of-range DECIMAL precision/scale from a malformed footer must return an error +// Status, not abort the process via decimal128()'s ARROW_CHECK. +TEST(TestAdapterRead, GetArrowTypeRejectsOutOfRangeDecimal) { + namespace orc_adapter = adapters::orc; + + EXPECT_THAT(orc_adapter::GetArrowType( + liborc::createDecimalType(Decimal128Type::kMaxPrecision + 1, 2).get()), + Raises(StatusCode::TypeError, testing::HasSubstr("decimal precision"))); + // Above INT_MAX: rejected on the raw uint64, not wrapped into range by static_cast. + EXPECT_THAT( + orc_adapter::GetArrowType(liborc::createDecimalType(uint64_t{1} << 40, 2).get()), + Raises(StatusCode::TypeError, testing::HasSubstr("decimal precision"))); + EXPECT_THAT(orc_adapter::GetArrowType(liborc::createDecimalType(10, 20).get()), + Raises(StatusCode::TypeError, testing::HasSubstr("decimal scale"))); + + // precision == 0 is the legacy HIVE 0.11/0.12 "max precision" sentinel. + ASSERT_OK_AND_ASSIGN(auto hive_sentinel, + orc_adapter::GetArrowType(liborc::createDecimalType(0, 0).get())); + AssertTypeEqual(*decimal128(38, 6), *hive_sentinel); + + ASSERT_OK_AND_ASSIGN(auto ok_mid, + orc_adapter::GetArrowType(liborc::createDecimalType(18, 4).get())); + AssertTypeEqual(*decimal128(18, 4), *ok_mid); + ASSERT_OK_AND_ASSIGN( + auto ok_max, + orc_adapter::GetArrowType(liborc::createDecimalType(Decimal128Type::kMaxPrecision, + Decimal128Type::kMaxPrecision) + .get())); + AssertTypeEqual(*decimal128(38, 38), *ok_max); +} + // Trivial class TestORCWriterTrivialNoWrite : public ::testing::Test {}; diff --git a/cpp/src/arrow/adapters/orc/util.cc b/cpp/src/arrow/adapters/orc/util.cc index c926998b0e17..85dd2ed0862e 100644 --- a/cpp/src/arrow/adapters/orc/util.cc +++ b/cpp/src/arrow/adapters/orc/util.cc @@ -1197,13 +1197,23 @@ Result> GetArrowType(const liborc::Type* type) { case liborc::DATE: return date32(); case liborc::DECIMAL: { - const int precision = static_cast(type->getPrecision()); - const int scale = static_cast(type->getScale()); + // decimal128() aborts via ARROW_CHECK when precision is out of [1, 38], so validate + // the untrusted footer values (as raw uint64, before the narrowing cast) first. + const uint64_t precision = type->getPrecision(); + const uint64_t scale = type->getScale(); if (precision == 0) { // In HIVE 0.11/0.12 precision is set as 0, but means max precision return decimal128(38, 6); } - return decimal128(precision, scale); + if (precision > static_cast(Decimal128Type::kMaxPrecision)) { + return Status::TypeError("Invalid ORC decimal precision ", precision, + ": must be in [1, ", Decimal128Type::kMaxPrecision, "]"); + } + if (scale > precision) { + return Status::TypeError("Invalid ORC decimal scale ", scale, + ": must be in [0, precision=", precision, "]"); + } + return decimal128(static_cast(precision), static_cast(scale)); } case liborc::LIST: { if (subtype_count != 1) {