google / google/s2geometry

EncodedS2ShapeIndex: NULL deref in Iterator::cell() on malformed input (DoS)

Open
#674 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.