Skip to content

fix(ipc): validate message framing bounds#955

Open
fallintoplace wants to merge 1 commit into
apache:mainfrom
fallintoplace:fix/ipc-framing-bounds
Open

fix(ipc): validate message framing bounds#955
fallintoplace wants to merge 1 commit into
apache:mainfrom
fallintoplace:fix/ipc-framing-bounds

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

Malformed IPC framing fields can currently reach allocations, FlatBuffer access, or mapped-file slicing before their ranges are validated. A stream can also advertise a huge but addressable body and trigger the allocation before truncation is discovered.

Refs #948

What changes are included in this PR?

  • Add configurable IPC metadata and body size limits, with defaults of 64 MiB and 256 MiB respectively.
  • Allow callers to disable either limit explicitly by setting it to zero.
  • Reject invalid stream metadata and body lengths before allocation.
  • Validate file block offsets, sizes, checked arithmetic, and continuation-prefix space for regular and mapped readers.
  • Fix the upper-bound check in RecordBatchAt.

Are these changes tested?

Yes. The IPC package tests cover oversized and signed metadata prefixes; negative, non-addressable, and huge-but-addressable body lengths; negative and overflowing block fields; out-of-file ranges; valid block bounds; and the zero-value body-limit opt-out. A rejecting test allocator ensures oversized body regressions fail without attempting a large allocation. IPC tests, race tests, vet, repeated regressions, and 32-bit compilation pass.

Are there any user-facing changes?

Malformed or over-limit IPC input now returns an error instead of reaching an allocation or panic. Callers that intentionally use unusually large messages can override the limits with WithMetadataSizeLimit and WithBodySizeLimit; passing zero disables the corresponding limit.

@fallintoplace fallintoplace requested a review from zeroshade as a code owner July 15, 2026 14:21

@zeroshade zeroshade left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This meaningfully hardens IPC framing — the metadata-length cap, negative/overflow rejection, and file-block validation are all good, and the RecordBatchAt off-by-one fix is correct. One gap: the stream body-length limit is effectively disabled by default, so a malformed huge body length still triggers a large allocation before truncation is detected. Detail inline.

Comment thread arrow/ipc/message.go
if bodyLen < 0 || bodyLen > maxInt {
return nil, fmt.Errorf("arrow/ipc: invalid message body length %d", bodyLen)
}
if r.maxBodySize > 0 && bodyLen > r.maxBodySize {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default maxBodySize is math.MaxInt (set in ipc.go), so on 64-bit a malformed stream advertising a huge-but-addressable BodyLength (e.g. 1<<40) passes this check and reaches body.Resize(int(bodyLen)) at L260 — a ~1 TiB allocation attempt — before io.ReadFull can discover the stream is truncated. That's an allocation-based DoS from a single malformed message on the default reader.

Suggest a sane default cap (cf. #953, which defaults to 64 MiB compressed / 256 MiB uncompressed) with an explicit opt-out for 0, and/or bounding the allocation to the bytes actually available. Also worth adding tests for negative BodyLength, BodyLength > MaxInt, and a huge-but-addressable body over the default limit — the current tests cover metadata-length prefixes and file blocks but not stream body rejection.

@fallintoplace fallintoplace force-pushed the fix/ipc-framing-bounds branch from aea406e to 849c4b2 Compare July 16, 2026 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants