Stack buffer overflow in `q3c_in_poly()` for large polygons

[demitri]

I was updating my qlsc project with the updated v2.0.5 q3c when I thought I'd run Claude Code against it to look for bugs. A few simple ones turned up. The rest of the message below is authored by Claude. -- Demitri

Summary

q3c_in_poly() / q3c_poly() (via q3c_check_sphere_point_in_poly) writes past the end of two stack/struct buffers when given a polygon large enough that its projected bounding box overflows 3 or 4 sides of a cube face. This is an out-of-bounds write, reproducible under AddressSanitizer, on version 2.0.5.

Details

Two coupled buffers are undersized, both on the point-in-polygon code path:

1. points[4] is too small — q3c_poly.c:323

   q3c_coord_t points[4];   /* in q3c_check_sphere_point_in_poly() */

   q3c_multi_face_check() (q3cube.c:434) writes 2 coordinates per overflowing face, up to 4 faces = 8 values. The other three callers (q3c_get_nearby, q3c_radial_query, q3c_poly_query) all declare points[8]; this one uses points[4]. ASan reports the OOB write at q3cube.c:470 (points[6]/points[7]).

2. xpj/ypj/axpj/aypj are dimensioned [3][…] — q3c.c:1028

   q3c_coord_t xpj[3][Q3C_MAX_N_POLY_VERTEX], ypj[3][...], axpj[3][...], aypj[3][...];

   The face loop at q3c_poly.c:365 runs face_count = 0 … multi_flag and indexes xpj[face_count], but multi_flag can be 3 or 4, so it indexes row 3/4 of a 3-row array. copy_q3c_poly_info_type() (q3c.c:1055) carries the same j < 3 limit.

Root cause

The "polygon too large" guard is both too permissive and checked too late:

• q3c_project_poly only sets large_flag when a vertex projects with cos(ra1) < Q3C_MINDISCR, i.e. a vertex roughly 90° from the face center — far looser than the documented ~23°-diameter limit.
• In pgq3c_in_poly the too_large error is raised at q3c.c:1304, after the vulnerable q3c_check_sphere_point_in_poly call at q3c.c:1301 has already run and overflowed.

So a polygon that should be rejected as too large instead reaches the projection loop with multi_flag = 3 or 4 and corrupts memory before it can be rejected.

(The range-query path q3c_poly_query is not affected — it reprojects per-face into a single buffer and uses a local points[8].)

Reproduction

SQL:

SELECT q3c_in_poly(10, 10, ARRAY[80,0, 0,80, 280,0, 0,-80]::double precision[]);

The vertices are each <90° from the face center (so large_flag does not fire in time) but the polygon spans enough sky to overflow all four face sides. Built with -fsanitize=address (standalone Q3C_STANDALONE build), this trips a stack-buffer-overflow; I observed multi_flag == 3 for this input.

ASan (with points[4]):

ERROR: AddressSanitizer: stack-buffer-overflow ... WRITE of size 8
  #0 q3c_multi_face_check q3cube.c:470
  #1 q3c_check_sphere_point_in_poly q3c_poly.c:361

After sizing points[8], ASan then reports the second overflow in the xpj[3][…] arrays.

Suggested fix

• q3c_poly.c:323: points[4] → points[8].
• q3c.c:1028: dimension the four projection arrays [5][Q3C_MAX_N_POLY_VERTEX] (main face + up to 4 secondary faces), and bump the copy_q3c_poly_info_type loop at q3c.c:1055 from j < 3 to j < 5.

Both changes together make the ASan report disappear. Ideally also tighten/relocate the size guard so oversized polygons are rejected before projection.

[demitri]

Follow-up from Demitri — a self-contained reproducer/test for this overflow. -- posted by Claude

Using the existing standalone (Q3C_STANDALONE) build; dump.c is produced by the make prepare step, so this links against the same objects the extension uses:

/* q3c_polyoverflow_test.c
 *   make prepare
 *   cc -O1 -g -fsanitize=address -DQ3C_STANDALONE -DQ3C_VERSION='"test"' \
 *       q3c_polyoverflow_test.c q3cube.c q3c_poly.c dump.c -lm -o q3c_polyoverflow_test
 *   ./q3c_polyoverflow_test
 */
#include "common.h"
extern struct q3c_prm hprm;

int main(void)
{
    /* Vertices are each < 90 deg from the main-face centre (so large_flag does
     * not fire in time), yet the polygon spans enough sky that its projected
     * bounding box overflows all four sides of the face -> multi_flag reaches
     * 3-4, past the points[4] bound. */
    q3c_coord_t ra[4]  = {80, 0, 280, 0};
    q3c_coord_t dec[4] = {0, 80, 0, -80};
    q3c_coord_t xpj[3][Q3C_MAX_N_POLY_VERTEX], ypj[3][Q3C_MAX_N_POLY_VERTEX];
    q3c_coord_t axpj[3][Q3C_MAX_N_POLY_VERTEX], aypj[3][Q3C_MAX_N_POLY_VERTEX];
    char faces[6], mf = 0, tl = 0;
    q3c_check_sphere_point_in_poly(&hprm, 4, ra, dec, 10.0, 10.0, &tl, 0,
                                   xpj, ypj, axpj, aypj, faces, &mf);
    return 0;
}

Under AddressSanitizer:

==ERROR: AddressSanitizer: stack-buffer-overflow ... WRITE of size 8
    #0 q3c_multi_face_check q3cube.c:470
    #1 q3c_check_sphere_point_in_poly q3c_poly.c:361
SUMMARY: AddressSanitizer: stack-buffer-overflow q3cube.c:470 in q3c_multi_face_check

A clean run (no ASan report) is the pass condition. Note that after sizing points[8], the same harness then trips the second overflow — the xpj/ypj/axpj/aypj arrays are dimensioned [3][...] but indexed up to multi_flag (3-4) — so both need addressing.