serhiy-bzhezytskyy opened a new pull request, #16478:
URL: https://github.com/apache/lucene/pull/16478
### Description
`LZ4#decompress` reads a match offset and checks only that it is not zero.
It does not check it against the number of bytes written so far, so an offset
larger than `dOff` makes the match reference bytes before `dest[0]` — bytes
this call never wrote:
```java
final int matchDec = compressed.readShort() & 0xFFFF;
if (matchDec == 0) {
throw new IOException("offset 0 is invalid");
}
...
for (int ref = dOff - matchDec, end = dOff + matchLen; dOff < end; ++ref,
++dOff) {
dest[dOff] = dest[ref]; // ref can be negative
}
...
System.arraycopy(dest, dOff - matchDec, dest, dOff, fastLen); // same
```
Corrupt input therefore fails with `ArrayIndexOutOfBoundsException:
arraycopy: source index -8 out of bounds for byte[18]` rather than a checked
`IOException`.
The LZ4 block format leaves this to the implementation, and @jpountz
described the same condition on LUCENE-5267 in 2013 while diagnosing an
`AIOOBE` from a corrupt `.fdt`:
> all the lines you pasted make no sense since `matchDec` should be lower
than `dOff`
The compressor already asserts the invariant when writing a match — `assert
matchDec > 0 && matchDec < 1 << 16` at `LZ4.java:188` — so this only affects
input that was not produced by `LZ4#compress`, which is to say corrupt or
hostile input.
### What this does and does not fix
It makes the failure legible. It does not improve corruption detection, and
it is worth being explicit about that, because the two are easy to conflate.
Measured on `main`, flipping one byte at a time across the `.fdt` of a
2,000-document index (codec and mode pinned to `Lucene104`/`BEST_SPEED`, every
37th byte, all documents read back and compared):
| outcome | `main` | with this change |
|---|---|---|
| `ArrayIndexOutOfBoundsException` from LZ4 | 36.1% | **0%** |
| `IOException` | 0.3% | **35.8%** |
| incorrect stored-field value returned, no error at all | 48.5% | 47.7% |
| `CorruptIndexException` | 0.8% | 0.6% |
| `NullPointerException` (header or index corruption) | 8.8% | 9.1% |
So the `AIOOBE` becomes a checked exception almost one-for-one, and the
largest outcome — a wrong document returned silently — is untouched. A validity
check cannot detect a valid-but-wrong LZ4 stream; most flipped bytes still
decode to something, just not to what was compressed. Only a checksum can catch
that, which is what LUCENE-5267 proposed and what the frame format specifies as
an optional per-block checksum.
That sampling is coarse: 2.7% of positions, all eight bits inverted rather
than one, only the first exception classified, one corpus and one compression
mode. It is offered as the shape of the change, not as a precise figure.
### Relationship to #15570
`matchDec == 0` was closed there in January. This is the other unchecked
path in the same method. `lucene-core` is not affected by CVE-2025-66566 — that
is a `lz4-java` advisory, and Lucene has its own vendored LZ4 — but the
advisory describes the general shape as *"an attacker can direct such a copy at
a region that hasn't been filled with decompressed bytes yet"*, and cites it as
prior art for why the bound matters. Whether Lucene's stored-fields path reuses
output buffers such that another document's bytes could surface is not
something I measured, so this is proposed as format conformance and error
legibility, not as a security fix.
### Verification
- Both new tests fail without the change, with `AIOOBE: source index -8` and
`source index -1`
- Removing the check and re-running (mutation check) fails; restoring it
passes
- The dictionary case is covered separately: with a preset dictionary the
bound is `dOff` rather than the number of bytes this call decompressed, since a
match may legitimately reference the dictionary the caller placed in
`dest[dOff-dictLen:dOff]`
- `:lucene:core:test` for `*LZ4*`, `*Compressing*` and `*StoredFields*`
passes with `-Ptests.nightly=true -Ptests.iters=5` (1,375 tests)
- `:lucene:core:check` and `tidy` pass
The `CHANGES.txt` entry uses `GITHUB#PENDING`; I will replace it with this
PR's number.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
