Naked ewkb from postigs (queried through python) causes OGR to segfault

[hobu]

ogr.CreateGeometryFromWkb('0103000020E610000001000000050000000000000000003440000000000000000000000000000034400000000000004940000000000000544000000000000049400000000000005440000000000000000000000000000034400000000000000000'.decode('hex'))

[warmerdam]

Mateusz,

Could you dig into this?  We don't need to support EWKB but do need to make
the WKB parser bulletproof in the face of corrupt input. 

As appropriate also please extend the test suite. 

[Mateusz Łoskot]

I checked this input binary stream with OGR and I'm not sure but is this possible this stream is broken?

The stream is 97 bytes long.
OGR identifies it as a polygon (using createFromWkb).
Next, OGRPolygon::importFromWkb calls following line:

memcpy( &nRingCount, pabyData + 5, 4 );

and output nRingCount gets value 4326

This value as a number of rings makes the crash, becuse the look goes outside the bytes stream.

Now, I'm trying to guess how to detect EWKB to return report it to user with appropriate error code.
I'm trying to guess, byt this valuye 4326 can be a SRID value because it is EPSG code for WGS 84.
As I understand from PostGIS docs
(http://postgis.refractions.net/docs/ch04.html#id2522740)
SRID in EWKB is placed just after the geometry type code.
But this can complicate the problem because I'm not sure how to detect this is EPSG and not number or rings. Both are 32 bit integers, right?

Also, PostGIS docs seems to me not very exact. In the following example:

SELECT 'SRID=4;POINT(0 0)'::geometry;

this EWKB is returned:

01010000200400000000000000000000000000000000000000
-------^^^

and what's this byte 20 is?
Despite of the question, this example makes me to think that my assumption about storing SRID in the EWKB is correct.

Frank, I'd need a piece of advice regarding the EWKB detection.

[warmerdam]

Mateusz, 

I think we will need to try and carry around, and check, the buffer length
to avoid going over the end.  Other than that I don't believe there is any
way to know in advance if the wkb is ewkb or not.

[Mateusz Łoskot]

Frank,

Yes, checking (or better estimating) buffer length that's the only solution I see.
Shortly, if we expect we can read SRID value from EWKB as a number of rings in the Polygon, then we can do test like this:

"if buffer length is at least as big as N * minRingSize"

where N is a SRID value read as number of rings (e.g. 4326)
and minRingSize is a size of minimal ring, in bytes (3 x point) what gives us
1 point = 16 bytes (raw point)
3 points = 48 bytes
so, minRingSize = 48 bytes

Final calculation is:
4326 rings * 48 bytes = 207648 bytes (minimum size of buffer).

Frank, does this estimation make sense for you?

[warmerdam]

Mateusz, 

Rather than using such estimates, is it possible to actually check that
we have enough buffer left as each ring is extracted?  So I imagine it
would mean the OGRLinearRing _importFromWkb() method would need to be
carefully checking against the amount of buffer left. 

[Mateusz Łoskot]

Tests of fixed EWKB handling in OGR

[Mateusz Łoskot]

New version of EWKB Tests

[Mateusz Łoskot]

(In reply to comment #7)
> Created an attachment (id=382) [edit]
> New version of EWKB Tests
> 
> New version provides LINESTRING test case.

Complete output of the test_ewkb application I get on my box:

mloskot:~/dev/gdal/bugs/1195$ ./test_ewkb
TEST SUITE 1
POINT (0 0)
POINT (0 0)
--- OK
TEST SUITE 2
POINT (15.122999999999999 21.456)
POINT (15.122999999999999 21.456)
--- OK
TEST SUITE 3
POINT (0 0 0)
POINT (0 0 0)
--- OK
TEST SUITE 4
POINT (52 21)
POINTM(52 21 75)
EWKB Z is Zero:
        0.000000 != 75.000000
--- OK
TEST SUITE 5
Failed to import from EWKB but no crash - GOOD!
Input EWKT: MULTIPOINTM(0 0 0,1 2 1)
--- OK
TEST SUITE 6
Failed to import from EWKB but no crash - GOOD!
MULTILINESTRING ((0 0 0,1 1 0,1 2 1),(2 3 1,3 2 1,5 4 1))
Input EWKT:
--- OK
TEST SUITE 7
Failed to import from EWKB but no crash - GOOD!
POLYGON ((0 0 0,4 0 0,4 4 0,0 4 0,0 0 0),(1 1 0,2 1 0,2 2 0,1 2 0,1 1 0))
Input EWKT:
--- OK
TEST SUITE 8
Failed to import from EWKB but no crash - GOOD!
MULTIPOLYGON (((0 0 0,4 0 0,4 4 0,0 4 0,0 0 0),(1 1 0,2 1 0,2 2 0,1 2 0,1 1 0)),((-1 -1 0,-1 -2 0,-2 -2 0,-2 -1 0,-1 -1 0)))
Input EWKT:
--- OK
TEST SUITE 9
POINT (52 21 75)
Input EWKT: POINT(52 21 75 100)
OGRGeometryFactory::createFromWkt FAILS on EWKT input - GOOD!
--- OK
TEST SUITE 10
POINT (0.0 0.0 0)
Input EWKT: SRID=4326;POINT(0 0 0)
Failed import of input EWKT - GOOD!
--- OK
TEST SUITE 11
POINT (0.0 0.0)
Input EWKT: SRID=4326;POINTM(52 21 75)
--- OK
TEST SUITE 12
Failed to import from EWKB but no crash - GOOD!
Input EWKT: SRID=4326;MULTIPOINTM(0 0 0,1 2 1)
--- OK
TEST SUITE 13
Failed to import from EWKB but no crash - GOOD!
Input EWKT: SRID=4326;MULTILINESTRING((0 0 0,1 1 0,1 2 1),(2 3 1,3 2 1,5 4 1))
--- OK
TEST SUITE 15
Failed to import from EWKB but no crash - GOOD!
Input EWKT: SRID=4326;MULTIPOLYGON(((0 0 0,4 0 0,4 4 0,0 4 0,0 0 0),(1 1 0,2 1 0,2 2 0,1 2 0,1 1 0)),((-1 -1 0,-1 -2 0,-2 -2 0,-2 -1 0,-1 -1 0)))
--- OK
TEST SUITE 16
LINESTRING (2 3,3 2,5 4)
Input EWKT: SRID=4326;LINESTRING((0 0 0,1 1 0,1 2 1))
--- OK
TEST SUITE 14
        IMPORTANT: If you see "Out of memory" error, that's GOOD!
ERROR 2: CPLCalloc(): Out of memory allocating 8388608 bytes.

Aborted
mloskot:~/dev/gdal/bugs/1195$

Note, that the "out of memory" error also occurs when querying my test PostgreSQL + PostGIS database attached to the Bug 1230, and that's an expected behaviour.
Any idea how to catch this bug and return e.g. OGRERR_* code?

[Mateusz Łoskot]

I've just commited changes fixing this bug.

[warmerdam]

Mateusz,

Do we still get this CPLCalloc() out of memory error?  I'm not sure why that occurs. 

[Mateusz Łoskot]

(In reply to comment #10)
> Mateusz,
> 
> Do we still get this CPLCalloc() out of memory error?  I'm not sure why that
> occurs. 

Frank,

I just fixed this issue. Now, it should reaturn OGRERR_NOT_ENOUGH_DATA code instad of throwing "out of memory" error.