Skip to content

Commit e3c51de

Browse files
authored
Fix overflow bug and missing return in Polygon encodeCompressed. (#271)
This change also adds compressed encodings to the fuzz test which would have allowed it to catch this.
1 parent 1c5af96 commit e3c51de

2 files changed

Lines changed: 43 additions & 3 deletions

File tree

s2/polygon.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1189,9 +1189,10 @@ func (p *Polygon) decodeCompressed(d *decoder) {
11891189
}
11901190
// Polygons with no loops are explicitly allowed here: a newly created
11911191
// polygon has zero loops and such polygons encode and decode properly.
1192-
nloops := int(d.readUvarint())
1192+
nloops := d.readUvarint()
11931193
if nloops > maxEncodedLoops {
11941194
d.err = fmt.Errorf("too many loops (%d; max is %d)", nloops, maxEncodedLoops)
1195+
return
11951196
}
11961197
p.loops = make([]*Loop, nloops)
11971198
for i := range p.loops {

s2/polygon_test.go

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,14 +1207,49 @@ func TestPolygonInvert(t *testing.T) {
12071207
// TestCloselySpacedEdgeVerticesKept
12081208
// TestPolylineAssemblyBug
12091209

1210+
func TestPolygonDecodeDoesNotOverflowForLargeNLoops(t *testing.T) {
1211+
var buf bytes.Buffer
1212+
e := encoder{w: &buf}
1213+
1214+
e.writeUint8(uint8(encodingPolygonCompressedVersion))
1215+
e.writeUint8(0) // snap level 0
1216+
// nloops: large value that overflows to negative when cast to int.
1217+
e.writeUvarint(1 << 63)
1218+
1219+
if e.err != nil {
1220+
t.Fatal(e.err)
1221+
}
1222+
1223+
var p Polygon
1224+
if err := p.Decode(bytes.NewReader(buf.Bytes())); err == nil {
1225+
t.Fatalf("Decode(overflow nloops) got nil error, want non-nil")
1226+
}
1227+
}
1228+
12101229
// go test -fuzz=FuzzDecodePolygon github.com/golang/geo/s2
12111230
func FuzzDecodePolygon(f *testing.F) {
12121231
for _, p := range []*Polygon{near0Polygon, near01Polygon, near30Polygon, near23Polygon, far01Polygon, far21Polygon, south0abPolygon} {
1213-
buf := new(bytes.Buffer)
1214-
if err := p.Encode(buf); err != nil {
1232+
// Add lossless encoding.
1233+
var buf bytes.Buffer
1234+
if err := p.Encode(&buf); err != nil {
12151235
f.Errorf("error encoding %v: ", err)
12161236
}
12171237
f.Add(buf.Bytes())
1238+
1239+
// Add compressed encoding.
1240+
var bufCompressed bytes.Buffer
1241+
e := encoder{w: &bufCompressed}
1242+
1243+
vs := make([]xyzFaceSiTi, 0, p.numVertices)
1244+
for _, l := range p.loops {
1245+
vs = append(vs, l.xyzFaceSiTiVertices()...)
1246+
}
1247+
1248+
p.encodeCompressed(&e, 0, vs)
1249+
if e.err != nil {
1250+
f.Errorf("error encoding compressed polygon: %v", e.err)
1251+
}
1252+
f.Add(bufCompressed.Bytes())
12181253
}
12191254

12201255
f.Fuzz(func(t *testing.T, encoded []byte) {
@@ -1223,6 +1258,10 @@ func FuzzDecodePolygon(f *testing.F) {
12231258
// Construction failed, no need to test further.
12241259
return
12251260
}
1261+
if err := p.Validate(); err != nil {
1262+
// Decoded but invalid polygon. Skip property checks.
1263+
return
1264+
}
12261265
if got := p.Area(); got < 0 {
12271266
t.Errorf("Area() = %v, want >= 0. Polygon: %v", got, p)
12281267
}

0 commit comments

Comments
 (0)