google / google/leveldb

ReadBlock allocates attacker-controlled snappy/zstd uncompressed length before validation

Open
#1,349 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
39.4k
Forks
8.2k
PR merge metrics
No merged PRs in 30d

Description

In `ReadBlock`'s decompression branches (table/format.cc at HEAD 7ee830d):

```cpp
case kSnappyCompression: {
size_t ulength = 0;
if (!port::Snappy_GetUncompressedLength(data, n, &ulength)) { ... }
char* ubuf = new char[ulength];
if (!port::Snappy_Uncompress(data, n, ubuf)) { ... }
```

`Snappy_GetUncompressedLength` only parses the leading varint of the block and accepts any value up to ~4 GiB; `new char[ulength]` executes before any decompression validation. The `kZstdCompression` branch has the identical pattern (`Zstd_GetUncompressedLength` followed by `new char[ulength]`).

**Reproduction** (90-byte crafted SSTable, no fuzzing required):

- data block whose entire contents are the 5 snappy header bytes `FE FF FF FF 0F` (declared uncompressed length 4294967294), trailer type `kSnappyCompression`; index block with a single entry pointing at that data block; standard footer with magic
- `Table::Open` succeeds; `NewIterator()` + `SeekToFirst()` triggers the block load

**Observed**: a single block read makes the process allocate ~4 GiB (VmPeak delta of +4.00 GiB on my machine) before `Snappy_Uncompress` fails and `Status::Corruption` is returned. Where the allocation exceeds the host's commit budget (containers with memory limits, `vm.overcommit_memory=0` with modest RAM+swap), `operator new` fails; since leveldb is built with `-fno-exceptions` (hardcoded in CMakeLists.txt), the failed allocation cannot be caught and terminates the entire host process (`std::bad_alloc` -> terminate -> SIGABRT). I verified this allocation-failure-to-abort behavior on the sibling allocation in the same function (`new char[n + kBlockTrailerSize]` with an oversized handle): SIGABRT, exit 134.

Multiple such blocks multiply the spike, and nothing bounds `ulength`. This is reachable whenever an application opens a crafted/restored/synced table. I reported this through Google's vulnerability process first; it was reviewed as a valid finding and I was directed to open it here.

**Suggested fix** — bound `ulength` before allocating, in both the snappy and zstd branches. For example, reject values above a configurable maximum decompressed block size, or values implausible relative to the compressed block size (a reasonable compression-ratio ceiling).

A self-contained reproduction program (crafts the table and drives `Table::Open`/`SeekToFirst`, printing VmPeak) is available; happy to attach it or send it as a PR.

Contributor guide

Open the contributing guide

Research direction

Start in table/format.cc at ReadBlock and inspect both the kSnappyCompression and kZstdCompression branches, then review the supplied Table::Open/NewIterator/SeekToFirst reproduction. Done means crafted compressed blocks cannot cause an attacker-controlled allocation before validation, and the failure returns Status::Corruption without terminating the process.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
databases, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.