Opened 7 years ago

Closed 7 years ago

#5655 closed defect (fixed)

[PATCH] Segfault in ogr2ogr when importing geojson to postgres

Reported by: rtorre Owned by: warmerdam
Priority: high Milestone: 1.11.1
Component: OGR_SF Version: 1.10.1
Severity: major Keywords: crash, segfault, geojson, PostgreSQL, pg,


For some datasets we found that ogr2ogr was crashing with a segfault. This is the stacktrace of the core dump:

(gdb) bt
#0  OGRPGEscapeStringList (hPGConn=0xe81e00, papszItems=0x0, bForInsertOrUpdate=0) at ogrpgtablelayer.cpp:1419
#1  0x00007f5ecdad9be7 in OGRPGTableLayer::CreateFeatureViaCopy (this=0x138ca40, poFeature=0x13aa030) at ogrpgtablelayer.cpp:1825
#2  0x0000000000409189 in TranslateLayer (psInfo=0x13ac1d0, poSrcDS=0x68cb30, poSrcLayer=0x1328d00, bTransform=1, bWrapDateline=0, pszDateLineOffset=0x40e485 "10", poOutputSRS=0x1393c80, bNullifyOutputSRS=0, poUserSourceSRS=0x0, poGCPCoordTrans=0x0, eGType=-2, bPromoteToMulti=1, nCoordDim=-1, eGeomOp=NONE, dfGeomOpParam=0, nCountLayerFeatures=0, poClipSrc=0x0, poClipDst=0x0, bExplodeCollections=0, nSrcFileSize=0, pnReadFeatureCount=0x0, pfnProgress=0, pProgressArg=0x0, poDstDS=<optimized out>) at ogr2ogr.cpp:3229
#3  0x0000000000405a38 in main (nArgc=<optimized out>, papszArgv=0x68d730) at ogr2ogr.cpp:2120

and this is specifically where it crashes:

static CPLString OGRPGEscapeStringList(PGconn *hPGConn,
                                       char** papszItems, int bForInsertOrUpdate)
    int bFirstItem = TRUE;
    CPLString osStr;
    if (bForInsertOrUpdate)
        osStr += "ARRAY[";
        osStr += "{";
    while(*papszItems) // <----------- NULL POINTER!!!
        if (!bFirstItem)
            osStr += ',';

        char* pszStr = *papszItems;
        if (*pszStr != '\0')
            if (bForInsertOrUpdate)
                osStr += OGRPGEscapeString(hPGConn, pszStr);

After a bit of investigation, we figured out that it was crashing because of a null pointer, and that situation happens when the input contains an empty array. See the attached files with a sample input and a small script to reproduce.

The patch is just adding a null check before executing the while loop, so it's pretty safe and produces the expected output. We'd be glad if you could integrate this patch in 1.11 as well as in trunk. Thanks!

Attachments (3)

fix-null-pointer-on-empty-array (2.0 KB) - added by rtorre 7 years ago.
Patch for segfault
profiles.minimal.geojson (706 bytes) - added by rtorre 7 years ago.
Example of dataset to reproduce the crash (588 bytes) - added by rtorre 7 years ago.
Small script to reproduce the issue

Download all attachments as: .zip

Change History (5)

Changed 7 years ago by rtorre

Patch for segfault

Changed 7 years ago by rtorre

Attachment: profiles.minimal.geojson added

Example of dataset to reproduce the crash

Changed 7 years ago by rtorre

Attachment: added

Small script to reproduce the issue

comment:1 Changed 7 years ago by rtorre

Reproduced on Ubuntu 12.04, gdal version 1.10.1 but also reproducible in 1.11. The patch should apply cleanly to both.

comment:2 Changed 7 years ago by Even Rouault

Resolution: fixed
Status: newclosed


trunk r27696, branches/1.11 r27697 "PG/PGDump: fix crash when writing a StringList? with 0 element (#5655, derived from patch by rtorre)"

Note: See TracTickets for help on using tickets.