Skip to content

fix(encoding): reject out-of-bounds variable-width offsets on decode - #8144

Merged
Xuanwo merged 2 commits into
mainfrom
xuanwo/lance-variable-width-offset-bug-1eb590
Aug 2, 2026
Merged

fix(encoding): reject out-of-bounds variable-width offsets on decode#8144
Xuanwo merged 2 commits into
mainfrom
xuanwo/lance-variable-width-offset-bug-1eb590

Conversation

@Xuanwo

@Xuanwo Xuanwo commented Aug 2, 2026

Copy link
Copy Markdown
Member

Variable-width block decoding trusted offsets that come straight from file bytes, so a file with corrupt offsets could be accepted by the default reader configuration: VariableWidthBlock::into_arrow used Arrow's unchecked builder unless the optional validate_on_decode flag was enabled, letting out-of-bounds offsets escape into Arrow arrays (returning bytes outside the values buffer, or crashing on access). The binary mini-block and block decompressors additionally sliced buffers with unvalidated offsets and header fields, which could panic the decode task.

This change makes layout validation mandatory at the Arrow conversion boundary: offsets must be monotonic and within the data buffer, and Utf8 values must be valid UTF-8. Violations surface as a typed CorruptFile error before any batch is returned, and validate_on_decode can no longer bypass the check — a private proof type ties the remaining unchecked build to the validation pass. The decompressors keep only the structural checks needed to avoid panics (plus the first-offset-must-be-zero block contract); offset values are validated once, at the conversion boundary, rather than rescanned per layer.

The memory-safety validation is unconditional by design, so the implementation keeps it off the critical path: the hot loop is a branchless monotonicity scan (monotonic + in-bounds endpoints imply all offsets are in bounds), and the mini-block check rides the existing rebase loop instead of adding a pass. Decode microbenchmarks on this change (dictionary strings, plain high-cardinality utf8 and binary, 1M rows) show no measurable throughput regression against the pre-fix baseline.

Supersedes #8143: its offset-value checks are covered by the conversion-boundary validation, and its structural checks (exact offsets-region size, zero first offset) are folded into the block decompressor here.

Variable-width decoding trusted file-derived offsets in three places:
`VariableWidthBlock::into_arrow` built Binary/Utf8 arrays with Arrow's
unchecked builder unless the optional `validate_on_decode` flag was set,
so corrupt offsets escaped into Arrow arrays (out-of-bounds reads or
crashes on access); the binary mini-block and block decompressors sliced
buffers with unvalidated offsets and header fields, which could panic the
decode task.

`into_arrow` now always validates the layout (offsets monotonic and in
bounds, values valid UTF-8 where required) and returns a typed
CorruptFile error; a private proof type ties the remaining unchecked
build to that validation pass.  The decompressors keep only the
structural checks needed to avoid panics, so offset values are validated
once, at the conversion boundary.  The hot loop is a branchless scan and
the mini-block check rides the existing rebase loop, leaving decode
throughput for valid data unchanged.
@github-actions github-actions Bot added bug Something isn't working A-encoding Encoding, IO, file reader/writer labels Aug 2, 2026
@Xuanwo
Xuanwo marked this pull request as ready for review August 2, 2026 17:47

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate recommendation: request changes. The central conversion validator correctly covers the malformed Arrow layouts exercised here, but miniblock decoding still lacks a structural boundary that lets corrupted metadata become user data. Validate the value-region start before slicing and add 32/64-bit overlap cases; that preserves prefix reads while completing the corrupt-file contract.

})
.collect::<Vec<u64>>();
let last = offsets[num_offsets - 1];
if !is_monotonic || last as usize > data.len() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first offset can still point inside the offset table, so a corrupt miniblock is accepted and serialized offsets are returned as string data instead of CorruptFile. Before slicing, require first >= num_offsets * bytes_per_offset; use a lower bound rather than equality because prefix decoding can leave unrequested offsets before the values.

Reproducer

I added this regression test to the existing test module and ran cargo test -p lance-encoding gate_repro_miniblock_offset_overlaps_offset_table --lib on this head:

#[test]
fn gate_repro_miniblock_offset_overlaps_offset_table() {
    use crate::compression::MiniBlockDecompressor;

    let mut chunk = [0_u32, 21, 25, 30]
        .iter()
        .flat_map(|offset| offset.to_le_bytes())
        .collect::<Vec<_>>();
    chunk.extend_from_slice(b"alphabetagamma");
    chunk.resize(chunk.len().next_multiple_of(8), 0);

    let block = super::BinaryMiniBlockDecompressor::new(32)
        .decompress(vec![LanceBuffer::from(chunk)], 3)
        .expect("decoder currently accepts the overlapping offset");
    assert!(block.into_arrow(DataType::Utf8, false).is_err());
}

Expected the assertion to pass; observed it fail because into_arrow returned Ok. Add equivalent 32-bit and 64-bit coverage.

…t table

A first offset pointing inside the chunk's offset table passed the
monotonic and tail-bounds checks, so a corrupt chunk decoded the
serialized offsets as value bytes instead of failing.  Require the value
region to start past the requested offsets; a lower bound rather than
equality because a prefix read legitimately leaves unrequested offsets
before the values.  A tampered first offset landing past that bound is
not detectable at this layer (the chunk stores no redundant value count
to cross-check), so this narrows the corrupt-file window rather than
closing it.
@Xuanwo
Xuanwo merged commit d644e7a into main Aug 2, 2026
40 checks passed
@Xuanwo
Xuanwo deleted the xuanwo/lance-variable-width-offset-bug-1eb590 branch August 2, 2026 19:56
Xuanwo added a commit that referenced this pull request Aug 3, 2026
Backport the corruption-safety fixes from:
- #8138: #8138
- #8144: #8144

The v3.0 stable line still reaches the vulnerable VariableFullZip
length-prefix parser and unchecked variable-width offset paths through
the default reader.

Release-specific adaptation: use the v3.0 error-construction and
synchronous reader APIs; retain one native truncated-prefix regression
plus native V2.1/V2.2/V2.3 reader coverage, without main-only
async/lazy/RLE scaffolding.

Validation:
- cargo fmt --all: passed
- Targeted lance-encoding/lance-file corruption regression tests: passed
(1 + 12 + 1 + 1 + 5 + 6 tests)
- cargo clippy --all --tests --benches -- -D warnings: passed
- No manifests, lockfiles, Python extras, or workflows were changed on
this branch.
- The current linux-build failure is the known ethnum/current-nightly
E0512 tooling baseline; Python and cargo-deny failures are also baseline
exceptions. The standalone create-rc workflow has no needs on these
validation workflows, so no release artifact blocker was identified.
- Keep nightly/dependency maintenance separate from this focused
backport.

---------

Co-authored-by: m00dy <professor.moody@pm.me>
Xuanwo added a commit that referenced this pull request Aug 3, 2026
Backport the corruption-safety fixes from:
- #8138: #8138
- #8144: #8144

The v4.0 stable line still reaches the vulnerable VariableFullZip
length-prefix parser and unchecked variable-width offset paths through
the default reader.

Release-specific adaptation: use the v4.0 error-construction and
synchronous reader APIs; retain one native truncated-prefix regression
plus native V2.1/V2.2/V2.3 reader coverage, with the release-specific
reader conflict resolved minimally.

Validation:
- cargo fmt --all: passed
- Targeted lance-encoding/lance-file corruption regression tests: passed
(1 + 12 + 1 + 1 + 5 + 6 tests)
- cargo clippy --all --tests --benches -- -D warnings: passed
- Rust build, build-no-lock, linux-build, MSRV, and format checks:
passed.
- No manifests, lockfiles, Python extras, or workflows were changed on
this branch.
- Python and cargo-deny failures are baseline exceptions; the standalone
create-rc workflow has no needs on these validation workflows, so no
release artifact blocker was identified.
- Keep Python/dependency maintenance separate from this focused
backport.

---------

Co-authored-by: m00dy <professor.moody@pm.me>
Xuanwo added a commit that referenced this pull request Aug 3, 2026
Backport the corruption-safety fixes from:
- #8138: #8138
- #8144: #8144

The v5.0 stable line still reaches the vulnerable VariableFullZip
length-prefix parser and unchecked variable-width offset paths through
the default reader.

Release-specific adaptation: use the v5.0 error-construction and
synchronous reader APIs; retain one native truncated-prefix regression
plus native V2.1/V2.2/V2.3 reader coverage, without main-only
async/lazy/RLE scaffolding.

Validation:
- cargo fmt --all: passed
- Targeted lance-encoding/lance-file corruption regression tests: passed
(1 + 12 + 1 + 1 + 5 + 6 tests)
- cargo clippy --all --tests --benches -- -D warnings: passed
- Rust build-no-lock, MSRV, clippy, and format checks: passed;
linux-build remains the known ethnum/current-nightly E0512 tooling
baseline.
- No manifests, lockfiles, Python extras, or workflows were changed on
this branch.
- Python and cargo-deny failures are baseline exceptions; the standalone
create-rc workflow has no needs on these validation workflows, so no
release artifact blocker was identified.
- This branch retains the existing recovery history around the #8144
backport (duplicate application followed by revert); no history was
rewritten, and the final tree contains the intended fix.
- Keep nightly/dependency maintenance separate from this focused
backport.

---------

Co-authored-by: m00dy <professor.moody@pm.me>
Xuanwo added a commit that referenced this pull request Aug 3, 2026
Backport the corruption-safety fixes from:
- #8138: #8138
- #8144: #8144

The v6.1 stable line still reaches the vulnerable VariableFullZip
length-prefix parser and unchecked variable-width offset paths through
the default reader.

Release-specific adaptation: use the v6.1 error-construction and
asynchronous reader APIs; retain one native truncated-prefix regression
plus native V2.1/V2.2/V2.3 reader coverage, without main-only lazy/RLE
scaffolding.

Validation:
- cargo fmt --all: passed
- Targeted lance-encoding/lance-file corruption regression tests: passed
(1 + 12 + 1 + 1 + 5 + 6 tests)
- cargo clippy --all --tests --benches -- -D warnings: passed
- Rust build, build-no-lock, linux-build, MSRV, clippy, and format
checks: passed.
- The license-header-checker install failure is an upstream
dynamic-installer/tooling baseline; Python and cargo-deny failures are
also baseline exceptions. The standalone create-rc workflow has no needs
on these validation workflows, so no release artifact blocker was
identified.
- No manifests, lockfiles, Python extras, or workflows were changed on
this branch.
- Keep license/Python/dependency maintenance separate from this focused
backport.

---------

Co-authored-by: m00dy <professor.moody@pm.me>
Xuanwo added a commit that referenced this pull request Aug 3, 2026
Backport the corruption-safety fixes from:
- #8138: #8138
- #8144: #8144

The v7.1 stable line still reaches the vulnerable VariableFullZip
length-prefix parser and unchecked variable-width offset paths through
the default reader.

Release-specific adaptation: use the v7.1 error-construction and
asynchronous reader APIs; retain one native truncated-prefix regression
plus native V2.1/V2.2/V2.3 reader coverage, without main-only lazy/RLE
scaffolding.

Validation:
- cargo fmt --all: passed
- Targeted lance-encoding/lance-file corruption regression tests: passed
(1 + 12 + 1 + 1 + 5 + 6 tests)
- cargo clippy --all --tests --benches -- -D warnings: passed
- Rust build, build-no-lock, linux-build, MSRV, clippy, and format
checks: passed.
- Python and cargo-deny failures are baseline exceptions; the standalone
create-rc workflow has no needs on these validation workflows, so no
release artifact blocker was identified.
- No manifests, lockfiles, Python extras, or workflows were changed on
this branch.
- Keep Python/dependency maintenance separate from this focused
backport.

---------

Co-authored-by: m00dy <professor.moody@pm.me>
Xuanwo added a commit that referenced this pull request Aug 3, 2026
Backport the corruption-safety fixes from:
- #8138: #8138
- #8144: #8144

The v8.0 stable line still reaches the vulnerable VariableFullZip
length-prefix parser and unchecked variable-width offset paths through
the default reader.

Release-specific adaptation: use the v8.0 error-construction and
asynchronous reader APIs; retain one native truncated-prefix regression
plus native V2.1/V2.2/V2.3 reader coverage, without main-only lazy/RLE
scaffolding.

Validation:
- cargo fmt --all: passed
- Targeted lance-encoding/lance-file corruption regression tests: passed
(1 + 12 + 1 + 1 + 5 + 6 tests)
- cargo clippy --all --tests --benches -- -D warnings: passed
- Rust build-no-lock, MSRV, clippy, and format checks: passed. The final
linux-build log shows the untouched
'rust/lance-index/src/vector/utils.rs:313' test
'test_simple_index_nearest_centroid::case_2_f32' failed with '45 != 42'
(642 passed, 1 failed); this is outside the backport diff.
- Python and cargo-deny failures are baseline exceptions. The standalone
create-rc workflow has no needs on these validation workflows, so no
release artifact blocker was identified.
- No manifests, lockfiles, Python extras, or workflows were changed on
this branch.
- Keep the independent lance-index/Python/dependency maintenance
separate from this focused backport.

---------

Co-authored-by: m00dy <professor.moody@pm.me>
Xuanwo added a commit that referenced this pull request Aug 3, 2026
Backport the corruption-safety fixes from:
- #8138: #8138
- #8144: #8144

The v9.0 stable line still reaches the vulnerable VariableFullZip
length-prefix parser and unchecked variable-width offset paths through
the default reader.

Release-specific adaptation: use the v9.0 error-construction and
asynchronous reader APIs; retain one native truncated-prefix regression
plus native V2.1/V2.2/V2.3 reader coverage, without main-only lazy/RLE
scaffolding.

Validation:
- cargo fmt --all
- Targeted lance-encoding/lance-file corruption regression tests: passed
- cargo clippy --all --tests --benches -- -D warnings: passed

---------

Co-authored-by: m00dy <professor.moody@pm.me>
@wjones127 wjones127 added the critical-fix Bugs that cause crashes, security vulnerabilities, or incorrect data. label Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-encoding Encoding, IO, file reader/writer bug Something isn't working critical-fix Bugs that cause crashes, security vulnerabilities, or incorrect data.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants