fix(ipc): validate message framing bounds#955
Conversation
zeroshade
left a comment
There was a problem hiding this comment.
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.
| if bodyLen < 0 || bodyLen > maxInt { | ||
| return nil, fmt.Errorf("arrow/ipc: invalid message body length %d", bodyLen) | ||
| } | ||
| if r.maxBodySize > 0 && bodyLen > r.maxBodySize { |
There was a problem hiding this comment.
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.
aea406e to
849c4b2
Compare
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?
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
WithMetadataSizeLimitandWithBodySizeLimit; passing zero disables the corresponding limit.