Ticket #154: 0001-img-Fix-out-of-bounds-read-of-coordinate-system-in-v.patch

File 0001-img-Fix-out-of-bounds-read-of-coordinate-system-in-v.patch, 3.7 KB (added by Philip Schuchardt, 17 hours ago)

Fix for this issue

  • src/img.c 

    From 182202afc917f4de59820cd0b5637748d2769ee5 Mon Sep 17 00:00:00 2001
From: Philip Schuchardt <vpicaver@gmail.com>
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 b v03d:  
    13971397           if (real_len != title_len) {
    13981398               char * cs = title + real_len + 1;
    13991399               real_len += strlen(cs) + 1;
    1400                if (memcmp(cs, "+init=", 6) == 0) {
     1400               if (strncmp(cs, "+init=", 6) == 0) {
    14011401                   /* PROJ 5 and later don't handle +init=esri:<number> but
    14021402                    * that's what cavern used to put in .3d files for
    14031403                    * coordinate systems specified using ESRI codes.  We parse
    … … v03d:  
    14111411                    * EPSG:<number>.
    14121412                    */
    14131413                   char * p = cs + 6;
    1414                    if (p[4] == ':' && isdigit((unsigned char)p[5]) &&
    1415                        ((memcmp(p, "epsg", 4) == 0 || memcmp(p, "esri", 4) == 0))) {
     1414                   if ((strncmp(p, "epsg:", 5) == 0 || strncmp(p, "esri:", 5) == 0) &&
     1415                       isdigit((unsigned char)p[5])) {
    14161416                       p = p + 6;
    14171417                       while (isdigit((unsigned char)*p)) {
    14181418                           ++p;
    … … v03d:  
    14311431                           *p = '\0';
    14321432                       }
    14331433                   }
    1434                } else if (memcmp(cs, "+proj=", 6) == 0) {
     1434               } else if (strncmp(cs, "+proj=", 6) == 0) {
    14351435                   /* Convert S_MERC and UTM proj strings which cavern used
    14361436                    * to generate to their corresponding EPSG:<number> codes.
    14371437                    */
    14381438                   char * p = cs + 6;
    1439                    if (memcmp(p, "utm +ellps=WGS84 +datum=WGS84 +units=m +zone=", 45) == 0) {
     1439                   if (strncmp(p, "utm +ellps=WGS84 +datum=WGS84 +units=m +zone=", 45) == 0) {
    14401440                       int n = 0;
    14411441                       p += 45;
    14421442                       while (isdigit((unsigned char)*p)) {
    14431443                           n = n * 10 + (*p - '0');
    14441444                           ++p;
    14451445                       }
    1446                        if (memcmp(p, " +south", 7) == 0) {
     1446                       if (strncmp(p, " +south", 7) == 0) {
    14471447                           p += 7;
    14481448                           n += 32700;
    14491449                       } else {
    … … v03d:  
    14551455                        * might not.
    14561456                        */
    14571457                       if (*p == '\0' || strcmp(p, " +no_defs") == 0) {
    1458                            /* There are at least 45 bytes (see memcmp above)
     1458                           /* There are at least 45 bytes (see strncmp above)
    14591459                            * which is ample for EPSG: plus an integer.
    14601460                            */
    14611461                           SNPRINTF(cs, 45, "EPSG:%d", n);
    14621462                       }
    1463                    } 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) {
     1463                   } 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) {
    14641464                       p = p + 89;
    14651465                       /* Allow +no_defs to be omitted as it seems to not
    14661466                        * actually do anything with recent PROJ - cavern always