Skip to content

Commit adecb17

Browse files
authored
GH-49803: [C++][CI] Avoid aborting when fuzz-mutated IPC file has less batches (#49804)
### Rationale for this change The IPC file fuzzer can remove batches from the IPC file footer while the batches do exist physically in the embedded IPC stream. This causes problem in differential fuzzing, because comparing the results of the IPC stream reader against the IPC file reader then fails. This issue was found by OSS-Fuzz: https://issues.oss-fuzz.com/issues/503357759 ### What changes are included in this PR? Skip differential fuzzing against the IPC stream reader if the number of batches read from the IPC file footer is not equal. ### Are these changes tested? Yes, by existing fuzz regression file. ### Are there any user-facing changes? No. * GitHub Issue: #49803 Authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: Antoine Pitrou <antoine@python.org>
1 parent 2581991 commit adecb17

2 files changed

Lines changed: 18 additions & 11 deletions

File tree

cpp/src/arrow/ipc/reader.cc

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2867,19 +2867,26 @@ Status FuzzIpcFile(const uint8_t* data, int64_t size) {
28672867
auto maybe_stream_result = do_stream_read();
28682868
final_status &= maybe_stream_result.status();
28692869
if (maybe_stream_result.ok()) {
2870-
if (maybe_read_result->schema->Equals(maybe_stream_result->schema,
2871-
/*check_metadata=*/true)) {
2872-
// XXX: in some rare cases, an IPC file might read unequal to the enclosed
2873-
// IPC stream, for example if the footer skips some batches or orders the
2874-
// batches differently. We should revisit this if the fuzzer generates such
2875-
// files.
2876-
compare_result(*maybe_stream_result);
2877-
} else {
2878-
// The fuzzer might have mutated the schema definition that is duplicated
2879-
// in the IPC file footer, in which case the comparison above would fail.
2870+
if (!maybe_read_result->schema->Equals(maybe_stream_result->schema,
2871+
/*check_metadata=*/true)) {
2872+
// The fuzzer may have mutated the schema definition that is duplicated
2873+
// in the IPC file footer, in which case the comparison would fail.
28802874
final_status &= Status::TypeError(
28812875
"Schema mismatch between IPC stream and IPC file footer, skipping "
28822876
"comparison");
2877+
} else if (maybe_read_result->batches.size() !=
2878+
maybe_stream_result->batches.size()) {
2879+
// The footer of a fuzzer-mutated IPC file might have added or removed some
2880+
// batches that are physically present in the IPC stream. In this case we
2881+
// don't want to abort with a comparison failure.
2882+
// XXX There might be more elaborate cases where the fuzzer reorders
2883+
// batches in the IPC file without adding or removing any, which is going
2884+
// to be considerably more difficult to detect.
2885+
final_status &= Status::Invalid(
2886+
"Different number of batches between IPC stream and IPC file footer, "
2887+
"skipping comparison");
2888+
} else {
2889+
compare_result(*maybe_stream_result);
28832890
}
28842891
}
28852892
}

0 commit comments

Comments
 (0)