ranflarion opened a new issue, #10521: URL: https://github.com/apache/arrow-rs/issues/10521
**Describe the bug** `StreamReader` reserves the message body length before reading any of the bytes it describes, so a corrupted or truncated stream aborts the process instead of returning an error. In `MessageReader::maybe_next` (`arrow-ipc/src/reader.rs`): ```rust self.buf.resize(meta_len, 0); self.reader.read_exact(&mut self.buf)?; let message = crate::root_as_message(self.buf.as_slice())?; let mut buf = MutableBuffer::from_len_zeroed(message.bodyLength() as usize); self.reader.read_exact(&mut buf)?; ``` `bodyLength()` is an `i64` read out of the stream's own flatbuffer metadata. It is handed to the allocator before a single body byte is read, so an implausible value is an allocation request, not a parse error. Depending on the value the result is either an abort (`memory allocation of N bytes failed`, uncatchable, and in a host process it takes down everything else in it) or a panic from `LayoutError` inside `MutableBuffer::from_len_zeroed`. A negative `bodyLength` is also accepted, because `as usize` wraps it into a very large positive length rather than being rejected. `meta_len` has the same shape one line earlier, bounded only by `i32::MAX`, so a four-byte corruption reserves and zeroes up to 2 GiB before discovering the bytes were never there. **To Reproduce** ```rust let mut fbb = flatbuffers::FlatBufferBuilder::new(); let mut message = crate::MessageBuilder::new(&mut fbb); message.add_version(crate::MetadataVersion::V5); message.add_header_type(crate::MessageHeader::NONE); message.add_bodyLength(1 << 50); let root = message.finish(); fbb.finish(root, None); let metadata = fbb.finished_data(); let mut stream = Vec::new(); stream.extend_from_slice(&[0xff; 4]); stream.extend_from_slice(&(metadata.len() as i32).to_le_bytes()); stream.extend_from_slice(metadata); // no body follows StreamReader::try_new(std::io::Cursor::new(stream), None) ``` The body length is consumed before the header is interpreted, so the header type does not matter. I found this from the other direction, fuzzing a real workload rather than a crafted message: sweeping every single-bit flip over the framing region of genuine IPC blocks produced `memory allocation of 1125899907497992 bytes failed` and a `SIGABRT`. In that setting the blocks are shuffle data crossing disk and network, so a single flipped bit takes out the whole process rather than failing one read that the caller could retry. **Expected behavior** A malformed or truncated stream should produce an `ArrowError`. Message lengths are attacker-controlled in any deployment that reads IPC from a network or from storage it does not exclusively own, so they should bound an allocation only after the bytes behind them have arrived. **Additional context** `read_block` (used by the file reader) has the same shape at `arrow-ipc/src/reader.rs:875`, plus two `unwrap()`s on the block metadata, so it is likely worth the same treatment. I have kept the fix I am about to post to the streaming path so the change stays reviewable. This overlaps #9777, which targets the same lines for a different reason (removing the redundant zeroing). The two are compatible and the outcomes point the same way: neither the zeroing nor the reservation should happen ahead of the read. -- 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]
