Bump orc to reject an out-of-range stream column in a stripe footer - #123464
Conversation
A stripe footer can declare a stream for a column id that is not in the file's type tree. `orc::ReaderImpl::preBuffer` indexed the selected-column mask with that id, which failed the libc++ hardening assertion and aborted the server. Stripes are prebuffered only for remote reads, so the crash needed `s3` or a data lake table function; `file` and `url` read the same file normally. The library now throws `orc::ParseError`. The test reads a 569-byte ORC file, embedded in the test, whose stripe footer declares a stream for column 20 in a file with 5 columns, through `s3`. Related: ClickHouse/orc#29 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Workflow [PR], commit [dc70abd] Summary: ❌
AI ReviewSummaryThis PR bumps Findings
Final Verdict
LLVM Coverage ReportMeasured on commit dc70abd.
Changed lines: Uncovered code analysis did not run: No coverable C/C++ source files changed (contrib/ is excluded from coverage). Newly covered: +187 lines in 56 files (-140 lines lost coverage) · Details |
…oter_stream_column` The ORC reader prebuffers stripes only when `remote_filesystem_read_prefetch` is on, and CI randomizes it. With it off the corrupt stream column is never reached on the prebuffer path, so the expected error was missing. https://s3.amazonaws.com/clickhouse-test-reports/praktika.html?PR=123464&sha=748a716418ce7ac4cba4adb758efb40395ba229e&name_0=PR Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Predicate pushdown loads the row indexes through `RowReaderImpl::loadStripeIndex`, which indexed the selected-column mask with the stream column from the stripe footer without a bounds check, also for local files that are never prebuffered. Cover it in `05317_orc_corrupt_stripe_footer_stream_column` with a local `file()` read and `WHERE id >= 0`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Build profile diff (arm_release)Comparing ✅ No significant changes. Binary sizes
The official master build is compiled with Compile time of recompiled translation units7 translation units recompiled, 10 s compile time in total, 7 of them have a recent master baseline. |
…e log scan Stress worker 1 (`--database=test_1`) runs with `memory_tracker_fault_probability=0.05`. Work that the stress phase leaves behind keeps the settings of the query that created it and is executed again by the upgraded server after the restart: - a distributed DDL entry stores the initiator's changed settings (`DDLLogEntry`) and `DDLTaskBase::makeQueryContext` applies them, so a queued `BACKUP ... ON CLUSTER` of `test_1` replays with fault injection and `BackupsWorker` logs `Failed to make internal backup ... fault injected` at `<Error>`; - a pending `Distributed` batch is sent with its stored settings, and the receiving async-insert flush logs `AsynchronousInsertQueue: Failed insertion ... fault injected` at `<Error>` with an empty query id, so the existing `} <Error> executeQuery: Code:` entry does not cover it. Seen on three unrelated PRs in 90 days (ClickHouse#123464, ClickHouse#117943, ClickHouse#120725); in each job these lines were the only output of the scan. Nothing enables fault injection on the upgraded server itself, so the message can only come from carried-over stress work. Ignore it as a fixed string, the same string the stress smoke check already tolerates (`ci/jobs/scripts/stress/stress.py`). It is MemoryTracker's message for both injection sites and does not match a real `memory limit exceeded` error. The large DDL backlog in the ClickHouse#123464 job comes from the previous release (26.9) wedging its DDLWorker on `KILL PART_MOVE_TO_SHARD ... ON CLUSTER`, which ClickHouse#122132 fixed on master only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A stripe footer can declare a stream for a column id that is not in the file's type tree.
orc::ReaderImpl::preBufferindexed the selected-column mask with that id, which failed the libc++ hardening assertion and aborted the server. Stripes are prebuffered only for remote reads, so the crash neededs3or a data lake table function;fileandurlread the same file normally. The library now throwsorc::ParseError.The test reads a 569-byte ORC file, embedded in the test, whose stripe footer declares a stream for column 20 in a file with 5 columns, through
s3.Related: ClickHouse/orc#29
Related: ClickHouse/orc#31
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Fixed a possible out of bounds access when reading an ORC file.
Closes: #123450
Workflow [PR]
Sync PR [sync-upstream/pr/123464]
Version info
26.10.1.1345-master(included in26.10and later)