Repository navigation
Conversation
GeometryReader used per-ring end indices and point offsets taken from the .fgb file to index the xy/z/m coordinate arrays without checking them against the number of coordinates actually stored, so a ring end past the xy count read past the coordinate buffer. Record the coordinate count when xy is read and reject ranges that do not fit before allocating or copying.
…mpty Zeroing numpoints on a rejected ring still left the ring in the shapeObj with a NULL point array, and shapeObj consumers do not expect that: msIsOuterRing() indexes line[r].point[0..2] without looking at numpoints, so a crafted ring end turned into a NULL dereference in the WFS GML writer instead of the out-of-bounds read. Have readLineObj() report whether it read the range and let readPolygon()/readLineString() keep only the parts that were read.
data/flatgeobuf-ring-invalid.fgb is a one-feature polygon whose second ring end claims 595 coordinate pairs that are not in xy. A WFS GetFeature against it must return the one stored ring and drop the other, which it does not do before the fix: the read runs past the feature buffer (ASan heap-buffer-overflow read of size 16 in readLineObj). misc-testcase runs under ASan in CI, so this covers the same ground as the unit test did. The unit test is dropped because it needs shapeObj's constructor and GeometryReader::read(), neither of which is exported from the MapServer DLL, so unit_test failed to link on the AppVeyor build.
|
What is the effect of this bug? Does it crash the server, does it make the rendered maps bad, or what? |
|
AppVeyor was mine: the unit test needed Running it through the real reader path also showed my first version was wrong: it zeroed |
3 participants
What does this PR do?
GeometryReaderinsrc/flatgeobuf/geometryreader.cppbuilds ashapeObjfrom an.fgbfeature's coordinates. For a polygon the per-ring end indices come from the geometry'sends()array in the file, andreadLineObj()used them to walkm_xy(and the parallelz/marrays) without checking them against the number of coordinate pairs actually present inxy(). A ring end index past the coordinate count reads past the buffer;readPoint()had the same unchecked indexing. The flatbuffer is not verified, so those sizes are file-controlled.The reader now records the available coordinate-pair count when
xyis read and rejects any ring range that does not fit it (and anyz/marray shorter than the requested points) before allocating or copying. A rejected ring is dropped rather than kept as an empty one, becauseshapeObjconsumers do not expect empty parts:msIsOuterRing()indexesline[r].point[0..2]without looking atnumpoints. Keeping the check inreadLineObj/readPointcovers point, line, polygon and the multipoint/multiline/multipolygon variants that all funnel through the same code.What are related issues/pull requests?
Same neighbourhood as #7545, which bounded the property blob; this covers the geometry-decoding side.
AI tool usage
Tasklist
/msautotest(follow steps in Regression Testing)