EncodedS2ShapeIndex: NULL deref in Iterator::cell() on malformed input (DoS)
- Dominant language
- C++
- Stars
- 2.7k
- Forks
- 357
- Avg merge
- 11h 4m
- Merged PRs (30d)
- 1
Description
## NULL pointer dereference in `EncodedS2ShapeIndex::Iterator::cell()` on malformed input
Decoding a malformed `EncodedS2ShapeIndex` crashes with SIGSEGV (NULL deref), even though `Init()` returns success.
### Root cause
Cells are decoded lazily. When a cell's contents are malformed, `S2ShapeIndexCell::Decode()` returns `false`, so `GetCell()` returns `nullptr`:
```cpp
// src/s2/encoded_s2shape_index.cc
auto cell = make_unique();
Decoder decoder = encoded_cells_.GetDecoder(i);
if (!cell->Decode(num_shape_ids(), &decoder)) {
return nullptr; // decode failure => null
}
```
`Iterator::cell()` unconditionally dereferences it:
```cpp
// src/s2/encoded_s2shape_index.h:316-318
inline const S2ShapeIndexCell& EncodedS2ShapeIndex::Iterator::cell() const {
ABSL_DCHECK(!done());
return *index_->GetCell(cell_pos_); // NULL deref when GetCell()==nullptr
}
```
### Reproduction
Build `libs2` (release, ASan) and run the documented traversal on the 149-byte
input below. `Init()` returns `true`, then `it.cell()` dereferences a null pointer.
```cpp
Decoder decoder(data, size);
EncodedS2ShapeIndex index;
index.Init(&decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder)); // true
for (EncodedS2ShapeIndex::Iterator it(&index, S2ShapeIndex::BEGIN);
!it.done(); it.Next()) {
const S2ShapeIndexCell& cell = it.cell(); // SEGV
(void)cell.num_clipped();
}
```
Crash input (base64):
```
EDmHAykACDQQEAABDI5AxsXB7z+Jcwt+P8I6GgK2gbjWT7Y/l7YwB2iR7z9c3IQEEr/BPwKBwrjWT7a/AgED/wIAYbRsOgOd7T/i3IKfho7VP4lzC34aOsY/CRFj46865T//hOpwPtDhP////////98/KDCol30D7D//8r7xPSDaP5AGk8F9kNA/KOAIEAgFEwACALg=
```
ASan (release `-DNDEBUG`, `-fsanitize=address`):
```
ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000
#0 gtl::compact_array_base::size() const compact_array.h:221
#1 S2ShapeIndexCell::num_clipped() const s2shape_index.h:139
#2 main s2_repro.cc:21
```
### Suggested fix
`GetCell()` must not return a pointer that callers blindly dereference. Either
null-check in `Iterator::cell()` and other `GetCell()` callers, or make `Init()`
validate every cell's contents so malformed input is rejected with `false` up
front (preserving the "lazy decode is infallible" invariant).
(Note: a separate `ABSL_CHECK(IsValid())` abort in `S2Polyline::DecodeUncompressed()`
is debug-only via `FLAGS_s2debug` and is **not** part of this report.)
Contributor guide
Research direction
Start with src/s2/encoded_s2shape_index.cc and src/s2/encoded_s2shape_index.h, tracing Init(), GetCell(), S2ShapeIndexCell::Decode(), and Iterator::cell(). Run the documented 149-byte ASan reproduction first, then inspect other GetCell() callers. Done means malformed input no longer causes a null dereference while valid encoded indexes continue to decode and traverse.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 54/100