negexx opened a new issue, #10437:
URL: https://github.com/apache/arrow-rs/issues/10437
**Describe the bug**
`arrow_ipc::convert::get_data_type` (`arrow-ipc/src/convert.rs`, confirmed
at both 58.3.0 and 58.4.0) uses `panic!`/`unimplemented!()` for a wide range of
malformed-but-structurally-plausible flatbuffer schema fields instead of
returning a `Result`. Any code that reads an untrusted/corrupted Arrow IPC file
(a partially-written file after a crash mid-write, a corrupted disk, or a
hostile input) via `FileReader::try_new`/`.next()` can be made to panic the
whole process rather than get a decode error back.
We found this via `cargo-fuzz` targeting our own thin wrapper around
`arrow::ipc::reader::FileReader`, not by inspecting arrow-ipc source first —
the fuzzer found a crashing input in well under a minute of running against a
corpus of otherwise-valid IPC files. The panic on our minimized crash input is:
```
not implemented: Type NONE not supported
```
which comes from the catch-all arm at the end of `get_data_type`:
```rust
t => unimplemented!("Type {:?} not supported", t),
```
([convert.rs, arrow-ipc 58.3.0, line
514](https://github.com/apache/arrow-rs/blob/58.3.0/arrow-ipc/src/convert.rs#L514))
That's one of roughly 20 `panic!`/`unimplemented!()` sites inside this same
function alone (lines ~309-514 in 58.3.0) covering bitwidth/signedness
mismatches, unsupported float/date/timestamp/interval/duration units, missing
list/map/union children, unexpected decimal bit widths, and unexpected union
modes — all reachable the same way, from parsing the schema section of an IPC
file, before any record batch validation happens.
**To Reproduce**
Minimal working reproducer (Rust, arrow-ipc = "58.3.0" or "58.4.0"):
```rust
use std::io::Cursor;
use arrow_ipc::reader::FileReader;
fn main() {
let bytes = /* base64 below, decode with e.g. the `base64` crate or
`echo ... | base64 -d` */;
let _ = FileReader::try_new(Cursor::new(bytes), None); // panics instead
of returning Err
}
```
Minimized crash input (1210 bytes, libFuzzer-minimized, base64-encoded):
```
QVJST1cxAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAP/////4AAAAEAAAAAAACgAMAAoACQAEAAoAAAAQgAAAAAEEAAgACAAAAAQACAAAAAQAAAACAAAAlAAAAAQAAACE////IAAAAAwAAAAAAAAQYAAAAAEAAAAkAAAAAAAGAAgABAAGAAAAAwAAABAAFgAQAAAADwAEAAAACAAQAAAAGAAAABwAAAAAAAADGAAAAAAABgAIAAYABgAAAAAAAQAAAAAABAAAAGl0ZW0AAAAABgAAAHZlY3RvcgAAEAAUABAAAAAPAAQAAAAIABAAAAAYAAAAIAAAAAAAAAIcAAAACAAMAAQACwAIAAAAQAAAAAAAAAEAAAAAAgAAAGlkAAD/////+AAAABAAAAAMABoAGAAXAAQACAAMAAAAICAAAEABAAAAAAAAAAAAAAAAAAMEAAoAGAAMAAgABAAKAAAATAAAABAAAAACAAAAAAAAAAAAAAADAAAAAgAAAAAAAAAAAAAAAAAAAAIAAAAAAAAAAAAAAAAAAAAGAAAAAAAAAAAAAAAAAAAAAAAAAAUAAAAAAAAAAAAAAAEAAAAAAAAAQAAAAAAAAAAQAAAAAAAAAIAAAAAAAAAAAQAAAAAAAADAAAAAAAAAAAEAAAAAAAAAAAEAAAAAAAAYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAAAAAAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAA/wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEEEAABBBAAAQQQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD/////AAAAABQAAAAAAAAADAAUABIADAAIAAQADAAAAOwAAAAIAQAAEAAAAAAABAAIAAgAAAAEAAgAAAAEAAAAAgAAAJQAAAAEAAAAhP///yAAAAAMAAAAAAAAEGAAAAABAAAAJAAAAAAABgAIAAQABgAAAAMAAAAQABYAEAAAAA8ABAAAAAgAEAAAABgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEEEAABBBAAAAAAYAAAB2ZWN0b3IAABAAFAAQAAAADwAEAAAACAAQAAAAGAAAACAAAAAAAAACHAAAAAgADAAEAAsACAAAAEAAAAAAAAABAAAAAAIAAABpZAAAAQAAAEABAAAAAAAAAAEAAAAAAABAAQAAAAAAAAAAAAAAAAAAKAEAAEFSUk9XMQ==
```
Decode with:
```sh
echo '<paste above>' | base64 -d > crash.arrow
```
Then:
```rust
let file = std::fs::File::open("crash.arrow").unwrap();
let _ = arrow_ipc::reader::FileReader::try_new(file, None); // panics
```
**Expected behavior**
`FileReader::try_new` (and `.next()`, which hits the same schema-parsing
path for some malformed inputs) should return `Err(ArrowError::...)` for a
structurally-malformed-but-parseable flatbuffer schema, the same way it already
does for other categories of invalid IPC input. A panic means any downstream
consumer that reads externally-sourced or on-disk IPC files (very common — this
is the standard way to persist Arrow data) needs to wrap every read in
`catch_unwind` to avoid an untrusted-input DoS, which is easy to miss and easy
to get wrong (allocation failures and unbounded recursion inside the same
schema walk aren't catchable that way either).
**Additional context**
We're not in a position to send a fix upstream ourselves in the near term,
but wanted to flag it since it's a real, fuzzer-found panic-on-untrusted-input
class of bug rather than a hypothetical. Happy to share the full fuzz
corpus/harness if useful.
--
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]
