Nayana-Naik73 opened a new pull request, #55159:
URL: https://github.com/apache/arrow/pull/55159
### Rationale for this change
`DecimalRescale` (behind `Decimal{32,64,128,256}::Rescale`) looks up
`GetScaleMultiplier(abs(new_scale - original_scale))` in the
`kDecimal*PowersOfTen` tables, and the only bound on that index is a `DCHECK`.
`Decimal*::FromString` rejects a large negative parsed scale but lets a
positive one through, and the precision it reports only counts significant
digits. So a CSV cell `1E-41` read as `decimal128(10, 2)` parses as precision
1, scale 41, passes the precision check in `DecimalValueDecoder::Decode`, and
`Rescale(41, 2)` reads one entry past `kDecimal128PowersOfTen` (ASan reports a
global-buffer-overflow READ of size 16 with `basic_decimal.cc` built with
`NDEBUG`). `1E-100000` reads about 1.6 MB past the table. The JSON reader's
decimal converter and the default `cast(string -> decimal)` take the same path,
and the cast with `allow_decimal_truncate=true` calls
`ReduceScaleBy(parsed_scale - out_scale)` directly with the same unbounded
value. Debug builds abort on the `DCHECK` for
all of these.
### What changes are included in this PR?
- `DecimalRescale` returns `kRescaleDataLoss` when the scale delta exceeds
`kMaxScale`. No multiplier that large fits in the type, so any non-zero value
would overflow or lose digits; zero still rescales to zero.
- The truncating branch of `StringToDecimal` returns zero when more than
`kMaxScale` digits are dropped, which is the truncated result for any value the
type can hold.
The check sits in `DecimalRescale` rather than `FromString` because
`FromString` intentionally returns scales above `kMaxScale` (for example
`1.23E-8` as Decimal32 with scale 10 in `DecimalFromStringTest.SmallValues`).
### Are these changes tested?
Yes. New cases in the typed `Rescale` test and `Decimal256Test.Rescale`, in
`Cast.StringToDecimal` (safe and truncating) and in the CSV
`DecimalConversion.Basics` test. All of them abort on the `DCHECK` without the
fix. `arrow-utility-test`, `arrow-compute-scalar-cast-test`, `arrow-csv-test`
and `arrow-json-test` pass on an ASan/UBSan debug build.
### Are there any user-facing changes?
A decimal string whose scale is further than `kMaxScale` from the target
scale now fails with the usual rescale data-loss error (or truncates to zero
when `allow_decimal_truncate` is set) instead of reading out of bounds.
**This PR contains a "Critical Fix".** Untrusted CSV or JSON input, or a
string column passed to `cast`, triggers an out-of-bounds read of a global
table in release builds, and the garbage multiplier can then produce an
incorrect converted value.
### Was AI used for this PR?
In accordance to the [AI generation
guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code),
please disclose below whether and how AI was used in this PR.
**PR code and description written by:**
- [X] Human
- [x] AI
**Reviewed before submission by:**
- [x] Human
- [ ] AI
- [ ] Not reviewed
--
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]
