GH-50162: [C++][Parquet] Avoid int32 overflow in BitPackedRunDecoder::GetBatch offset

[metsw24-max]

int32 overflow in the bit-packed run decoder offset
GetBatch works out the byte position with values_read_ * value_bit_width in 32-bit int. For a large bit-packed run (this decodes untrusted parquet RLE/bit-packed dictionary indices and levels, with value width up to 64) the product passes INT32_MAX and wraps negative, so bytes_fully_read goes negative and unread_data ends up before the buffer, giving an out of bounds read in unpack. raw_data_size just above already widens to int64 before the same multiply, so I matched that here.

• GitHub Issue: [C++][Parquet] int32 overflow in BitPackedRunDecoder::GetBatch offset for large bit-packed runs #50162

[github-actions]

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - Contributing Overview

[pitrou]

@metsw24-max Thanks for submitting this PR! Can you open a corresponding GitHub issue as instructed by the comment bot above?

@AntoinePrv Would you like to review these changes?

[AntoinePrv]

I think ideally we would want to check if the untrusted int is within the bounds of the parquet specs, and if not error (here it may be return negative value?).

Though is the bit_width check done before? if not, should it?

[AntoinePrv]

In the same way that it may previously not fit in an int do we know that it will fit it at this point (and not turn negative)?

[metsw24-max]

For parser-produced runs it does hold: PeekImpl only emits a run whose whole payload fits in the remaining buffer (it truncates or rejects the run otherwise), and the BitPackedRun constructor DCHECKs that invariant. Since values_read_ <= values_count_, bytes_fully_read <= max_read_bytes_, so the difference stays in [0, max_read_bytes_], which is itself an rle_size_t. The remaining case is the negative sentinel (no bound), where the difference stays negative and unpack treats any negative value the same as -1. Added a DCHECK at the subtraction site to make that explicit.

[github-actions]

⚠️ GitHub issue #50162 has been automatically assigned in GitHub to PR creator.

[metsw24-max]

@pitrou done, opened GH-50162 and renamed the title to match.

@AntoinePrv on the spec bounds: the bit width is already checked before it reaches this decoder. Dictionary indices reject anything above 32 in DictDecoderImpl::SetData, and for rep/def levels the width isn't read from the file at all, it's derived from max_level. Run lengths are bounded by the parser, which truncates or rejects a run that would overflow the buffer. The catch is that the values overflowing here are all within spec: a single bit-packed run can validly hold close to 2^31 values, so values_read_ * value_bit_width passes INT32_MAX on legitimate data once a run grows past 256 MiB. So I don't think there's an out-of-spec value to error on at this level; the intermediate just needs the wider type, same as raw_data_size above. Happy to add an explicit error path if you'd rather have one.

[pitrou]

+1, thank you @metsw24-max .

@AntoinePrv Do you want to give this another review?

[AntoinePrv]

LGTM. Sorry about the slow response time.

Not sure if we should add a test with that many values to exercise that error path or if it is too demanding for the tests.

[pitrou]

Not sure if we should add a test with that many values to exercise that error path or if it is too demanding for the tests.

Probably too demanding I would say.

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit a2594df.

There was 1 benchmark result indicating a performance regression:

• Commit Run on test-mac-arm at 2026-06-16 18:14:38Z
  • tpch (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-21, scale_factor=1

The full Conbench report has more details. It also includes information about 12 possible false positives for unstable benchmarks that are known to sometimes produce them.