diff --git a/src/s2/mutable_s2shape_index_test.cc b/src/s2/mutable_s2shape_index_test.cc index 101241bc..3cc798b7 100644 --- a/src/s2/mutable_s2shape_index_test.cc +++ b/src/s2/mutable_s2shape_index_test.cc @@ -44,6 +44,8 @@ #include "absl/types/span.h" #include "s2/base/log_severity.h" +#include "s2/encoded_s2cell_id_vector.h" +#include "s2/encoded_string_vector.h" #include "s2/r2.h" #include "s2/r2rect.h" #include "s2/s1angle.h" @@ -821,6 +823,32 @@ TEST_F(MutableS2ShapeIndexTest, DecoderCatchesInvalidIndex) { EXPECT_FALSE(ok); } +TEST_F(MutableS2ShapeIndexTest, DecoderCatchesOutOfRangeShapeId) { + // An encoded index whose single cell references a shape id (5000) far beyond + // the number of shapes in the index (2). Without validation, Init() accepts + // this and a later shape(clipped.shape_id()) lookup reads out of bounds. + MutableS2ShapeIndex shapes; + shapes.Add( + make_unique(vector{S2Point(1, 0, 0)})); + shapes.Add( + make_unique(vector{S2Point(0, 1, 0)})); + + Encoder encoder; + encoder.Ensure(64); + encoder.put_varint64(10 << 2 | 0); // max_edges_per_cell = 10, version = 0 + s2coding::EncodeS2CellIdVector({S2CellId::FromFace(0)}, &encoder); + s2coding::StringVectorEncoder cells; + Encoder* cell = cells.AddViaEncoder(); + cell->Ensure(8); + cell->put8(0x00); // contiguous range of edges, edge id 0 + cell->put_varint32(5000 << 4); // shape id delta 5000, num_edges 1 + cells.Encode(&encoder); + + Decoder decoder(encoder.base(), encoder.length()); + MutableS2ShapeIndex index; + EXPECT_FALSE(index.Init(&decoder, s2shapeutil::WrappedShapeFactory(&shapes))); +} + TEST(S2Shape, user_data) { struct MyData { int x, y; diff --git a/src/s2/s2shape_index.cc b/src/s2/s2shape_index.cc index a50426d8..8646773a 100644 --- a/src/s2/s2shape_index.cc +++ b/src/s2/s2shape_index.cc @@ -283,6 +283,11 @@ bool S2ShapeIndexCell::Decode(int num_shape_ids, Decoder* decoder) { clipped->set_contains_center((header & 4) != 0); if (!DecodeEdges(num_edges, clipped, decoder)) return false; } + + // Shape ids come from the encoded data, so reject any that are out of + // range. Otherwise a caller that indexes S2ShapeIndex::shape() with a + // decoded shape id reads out of bounds. + if (shape_id >= num_shape_ids) return false; } return true; }