Skip to content

geog_in: missing braces around WKT-parse failure branch causes NULL dereference when meos_error returns #1089

@estebanzimanyi

Description

@estebanzimanyi

Bug

In meos/src/geo/postgis_funcs.c (around line 3732), the WKT branch of geog_in is missing braces:

/* WKT then. */
else
{
  if ( lwgeom_parse_wkt(&lwg_parser_result, (char *) str, 
      LW_PARSER_CHECK_ALL) == LW_FAILURE )
    PG_PARSER_ERROR(lwg_parser_result);
    lwgeom = lwg_parser_result.geom;
}

PG_PARSER_ERROR (defined just above, line 63) expands to do { meos_error(ERROR, ...); } while(0); -- it does not return. Because of the missing braces, only the macro is the if-body; the assignment lwgeom = lwg_parser_result.geom; runs unconditionally. On parse failure, lwg_parser_result.geom is NULL (set by lwgeom_parser_result_init), so the subsequent if (lwgeom->srid == SRID_UNKNOWN || ...) at line 3739 dereferences NULL.

Why it usually doesn't crash in standalone MEOS

In standalone, no custom MEOS_ERROR_HANDLER is installed → meos_error(ERROR, ...) falls into the fprintf(stderr, ...) + exit(EXIT_FAILURE) branch at meos/src/temporal/error.c:220 and the process exits before the NULL deref. So the bug is hidden.

Why it surfaces in MobilityDuck (DuckDB extension)

MobilityDuck loads MEOS via meos_initialize() from within a long-lived DuckDB process. When meos_error(ERROR, ...) is invoked from inside a query worker, the exit(EXIT_FAILURE) in the default handler aborts the entire DuckDB process; depending on atexit/cleanup interactions (threads, stdio buffering, libc destructors), this can present to the user as a SIGSEGV rather than a clean exit. MobilityDuck's src/geo/stbox_functions.cpp:23-26,1146 documents working around exactly this class of failure ("avoids MEOS geog_in/geog_area which can SIGSEGV in this extension") with a permanent C++ spherical-lon/lat sidestep for stbox area.

Fix

Add the missing braces and make PG_PARSER_ERROR defensively return NULL after meos_error so a future custom handler that does not abort will not fall through:

/* Modified version of PG_PARSER_ERROR */
#if MEOS
#define PG_PARSER_ERROR(lwg_parser_result) \
  do { \
    meos_error(ERROR, MEOS_ERR_INTERNAL_TYPE_ERROR, \
      "%s", lwg_parser_result.message); \
    return NULL; \
  } while(0);
#else
  #include <lwgeom_pg.h>
#endif

and / or rewrite the WKT branch in geog_in with explicit braces:

else
{
  if (lwgeom_parse_wkt(&lwg_parser_result, (char *) str,
                       LW_PARSER_CHECK_ALL) == LW_FAILURE)
    PG_PARSER_ERROR(lwg_parser_result);
  lwgeom = lwg_parser_result.geom;
  if (! lwgeom)
    return NULL;
}

(The if (! lwgeom) is belt-and-suspenders in case a future change to PG_PARSER_ERROR does not return.)

Audit of other PG_PARSER_ERROR callsites

A grep for PG_PARSER_ERROR( should be run; every callsite that uses it without braces is potentially affected by the same fall-through. (Filed companion analysis on MobilityDuck#157 for the second related crash, geog_area / stbox_area on geodetic 3D / PolyhedralSurface, which is a separate MEOS bug deeper in the lwgeom polyhedral-area path -- this issue tracks only geog_in.)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions