Skip to content

fix(fsst): reject corrupt symbol tables and offsets on decode - #8588

Merged
Xuanwo merged 2 commits into
mainfrom
xuanwo/fsst-untrusted-decode
Aug 20, 2026
Merged

fix(fsst): reject corrupt symbol tables and offsets on decode#8588
Xuanwo merged 2 commits into
mainfrom
xuanwo/fsst-untrusted-decode

Conversation

@Xuanwo

@Xuanwo Xuanwo commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Why

#7589 made the FSST output-buffer contract 8×, but still trusted on-disk symbol lengths and value offsets. A crafted Lance file can inflate lens[] so decompress_bulk writes past that buffer. Readers that open untrusted datasets (dataset viewers, upload scanners) crash, and the overflow is a heap write with attacker-controlled values and stride.

This change makes fsst::decompress the security boundary. Declared symbol lengths must be 1..=8. Offsets must convert with to_usize, be non-decreasing, and stay inside the compressed buffer. Corrupt input returns InvalidData, mapped to corrupt_file by the encoding adapters. Valid files and the 8× write_unaligned fast path are unchanged.

Benchmark

FSST string decode of a 1 MiB Hamlet corpus, 2000 decompress calls, release, same host. The measured head is this PR; the baseline is origin/main (8a8fb20c32).

Workload Baseline This PR
Decode 1 MiB FSST strings 11.9 GB/s 11.7 GB/s

The difference is within run-to-run noise. Decode cost is unchanged.

Treat on-disk FSST symbol lengths and value offsets as untrusted input so a crafted table cannot overflow the 8x decode buffer.
@github-actions github-actions Bot added A-encoding Encoding, IO, file reader/writer bug Something isn't working labels Aug 17, 2026
@Xuanwo
Xuanwo marked this pull request as ready for review August 17, 2026 17:07
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-changes Latest Gatekeeper recommendation requests changes. label Aug 18, 2026
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@wjones127
wjones127 self-requested a review August 19, 2026 15:29

@westonpace westonpace left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changing back to request changes since gatekeeper's suggestions seem valid.

Reject corrupt offsets on the raw-copy path, and return InvalidData when an FSST_ESC is missing its payload inside the current value.
@lance-gatekeeper lance-gatekeeper Bot removed the K-changes Latest Gatekeeper recommendation requests changes. label Aug 20, 2026

@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: approve.

The revision closes both previously identified malformed-input paths: validation now covers switch-off offsets, and escape payloads are bounded to their current value. The decoder now enforces the intended corruption boundary while preserving valid-file behavior and the existing fast-path safety invariants.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 20, 2026
@Xuanwo
Xuanwo requested a review from westonpace August 20, 2026 13:23

@westonpace westonpace left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice job! Good to see a safe & fast impl 😄

@Xuanwo
Xuanwo merged commit d1e971c into main Aug 20, 2026
39 checks passed
@Xuanwo
Xuanwo deleted the xuanwo/fsst-untrusted-decode branch August 20, 2026 13:40
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 K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants