Arawoof06 opened a new pull request, #50965: URL: https://github.com/apache/arrow/pull/50965
### Rationale for this change `DecimalRescale` indexes the scale-multiplier table through `GetScaleMultiplier(abs_delta_scale)`, but that table only holds `kMaxScale + 1` entries and is guarded by a `DCHECK` that disappears under `NDEBUG`. `Decimal::FromString` clamps a negative parsed scale but leaves a positive one unbounded, so a literal like `1E-100` parses to precision 1 and scale 100. In the CSV decimal converter that passes the precision check and then calls `Rescale(100, 0)`, reading `kDecimal128PowersOfTen[100]` well past the end of the 39-entry table. The same path is reachable from the public `Decimal32/64/128/256::FromString` and `Rescale` APIs. UBSan flags it as `index 100 out of bounds for type 'const BasicDecimal128[39]'`. ### What changes are included in this PR? Reject a scale delta whose magnitude exceeds `kMaxScale` in `DecimalRescale` before the lookup, returning `kRescaleDataLoss`. A change that large can never be represented without overflow or truncation anyway. The check lives in the one templated function so it covers all four decimal widths, and testing the delta before `std::abs` also keeps `std::abs(INT32_MIN)` out of reach. ### Are these changes tested? Yes. The typed `Rescale` test now asserts a delta past `kMaxScale` fails in both directions (a delta of exactly `kMaxScale` is still exercised by the existing loops). A new `Decimal128` case parses `1E-100`, confirms scale 100, and checks the following `Rescale` returns Invalid instead of reading past the table. Built with `-fsanitize=bounds`, the prior code aborts at `GetScaleMultiplier`; with the fix it returns cleanly. ### Are there any user-facing changes? A decimal string whose scale exceeds the type maximum now returns an Invalid status from `Rescale` rather than reading out of bounds. Valid inputs are unchanged. **This PR contains a "Critical Fix".** An out-of-bounds read of a static table, reachable from untrusted decimal strings (for example CSV values), that the `DCHECK` bound does not catch in release builds. * GitHub Issue: #50964 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
