From 182202afc917f4de59820cd0b5637748d2769ee5 Mon Sep 17 00:00:00 2001 From: Philip Schuchardt Date: Fri, 11 Sep 2026 15:28:50 -0500 Subject: [PATCH] img: Fix out-of-bounds read of coordinate system in v8 .3d files A v8 .3d file written with a non-default separator but no coordinate system (any Walls import, for example) carries an empty coordinate system string after the title's NUL. Reading it back, the checks for "+init=" and "+proj=" used memcmp, which reads 6 bytes regardless of where the string ends; for a short header line the whole line fits in getline_alloc_len's initial 16 byte buffer, so the read runs past the allocation. The inner "epsg"/"esri" check had the same problem: it looked at p[4] and p[5] before knowing the string was that long. Bound every read by the terminating NUL by using strncmp, and match "epsg:"/"esri:" before testing the digit that follows. Strings that matched before match exactly as they did. tests/wallsdiving.srv already triggers this: running dump3d on the .3d cavern writes for it reports a heap-buffer-overflow under AddressSanitizer, so the sanitisers and valgrind CI jobs cover the fix. --- src/img.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/img.c b/src/img.c index 711efbbd..1901536e 100644 --- a/src/img.c +++ b/src/img.c @@ -1397,7 +1397,7 @@ v03d: if (real_len != title_len) { char * cs = title + real_len + 1; real_len += strlen(cs) + 1; - if (memcmp(cs, "+init=", 6) == 0) { + if (strncmp(cs, "+init=", 6) == 0) { /* PROJ 5 and later don't handle +init=esri: but * that's what cavern used to put in .3d files for * coordinate systems specified using ESRI codes. We parse @@ -1411,8 +1411,8 @@ v03d: * EPSG:. */ char * p = cs + 6; - if (p[4] == ':' && isdigit((unsigned char)p[5]) && - ((memcmp(p, "epsg", 4) == 0 || memcmp(p, "esri", 4) == 0))) { + if ((strncmp(p, "epsg:", 5) == 0 || strncmp(p, "esri:", 5) == 0) && + isdigit((unsigned char)p[5])) { p = p + 6; while (isdigit((unsigned char)*p)) { ++p; @@ -1431,19 +1431,19 @@ v03d: *p = '\0'; } } - } else if (memcmp(cs, "+proj=", 6) == 0) { + } else if (strncmp(cs, "+proj=", 6) == 0) { /* Convert S_MERC and UTM proj strings which cavern used * to generate to their corresponding EPSG: codes. */ char * p = cs + 6; - if (memcmp(p, "utm +ellps=WGS84 +datum=WGS84 +units=m +zone=", 45) == 0) { + if (strncmp(p, "utm +ellps=WGS84 +datum=WGS84 +units=m +zone=", 45) == 0) { int n = 0; p += 45; while (isdigit((unsigned char)*p)) { n = n * 10 + (*p - '0'); ++p; } - if (memcmp(p, " +south", 7) == 0) { + if (strncmp(p, " +south", 7) == 0) { p += 7; n += 32700; } else { @@ -1455,12 +1455,12 @@ v03d: * might not. */ if (*p == '\0' || strcmp(p, " +no_defs") == 0) { - /* There are at least 45 bytes (see memcmp above) + /* There are at least 45 bytes (see strncmp above) * which is ample for EPSG: plus an integer. */ SNPRINTF(cs, 45, "EPSG:%d", n); } - } else if (memcmp(p, "merc +lat_ts=0 +lon_0=0 +k=1 +x_0=0 +y_0=0 +a=6378137 +b=6378137 +units=m +nadgrids=@null", 89) == 0) { + } else if (strncmp(p, "merc +lat_ts=0 +lon_0=0 +k=1 +x_0=0 +y_0=0 +a=6378137 +b=6378137 +units=m +nadgrids=@null", 89) == 0) { p = p + 89; /* Allow +no_defs to be omitted as it seems to not * actually do anything with recent PROJ - cavern always -- 2.50.1 (Apple Git-155)