From e4c65a9ddca2c2f88bc311165fc440d9ee856cfd Mon Sep 17 00:00:00 2001 From: nurdmuny Date: Tue, 9 Jun 2026 06:41:29 -0700 Subject: [PATCH 1/2] FileOpen + InitInterProcessComm: bound user-input string parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two memory-safety fixes in the config-file ingest paths: 1. Kit/Source/iokit.c::FileOpen strcpy + strcat into a 1024-byte fixed buffer with no bound check on the inputs. A long working directory + long config filename passed through the same FileOpen() call overflows FileName[1024] on the stack. Replaced with snprintf + truncation check. 2. Source/42ipc.c::InitInterProcessComm Five fscanf calls used the unbounded '%s' / '%[^...]' conversion into fixed-size buffers (HostName[40], response[120], FileName[80], Prefix[80]). A hand-written or auto-generated Inp_IPC.txt with a field longer than the buffer overflows the stack frame. Added explicit width specifiers to every %s / %[^...] specifier so the conversion truncates instead of overrunning. These are the two functions on the IPC-config entry path; the same pattern recurs in 42init.c (LoadSC) and 42gl.c (Map / POV config parsers). I left those for a follow-up if you want this scope of change applied more broadly — happy to extend the PR. --- Kit/Source/iokit.c | 16 +++++++++++++--- Source/42ipc.c | 14 +++++++------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/Kit/Source/iokit.c b/Kit/Source/iokit.c index 5afb046c..ca1cbfb4 100755 --- a/Kit/Source/iokit.c +++ b/Kit/Source/iokit.c @@ -24,9 +24,19 @@ FILE *FileOpen(const char *Path, const char *File, const char *CtrlCode) { FILE *FilePtr; char FileName[1024]; - - strcpy(FileName,Path); - strcat(FileName,File); + int Written; + + /* Use snprintf instead of strcpy+strcat — the previous version + could overflow FileName[1024] when Path and File combined to + more than 1024 bytes (e.g. a long working directory plus a + long config-file name). Truncation is preferred over the + silent stack smash that strcpy+strcat caused. */ + Written = snprintf(FileName,sizeof(FileName),"%s%s",Path,File); + if(Written < 0 || (size_t)Written >= sizeof(FileName)) { + printf("Error: combined path length exceeds %zu bytes: %s%s\n", + sizeof(FileName)-1, Path, File); + exit(1); + } FilePtr=fopen(FileName,CtrlCode); if(FilePtr == NULL) { printf("Error opening %s: %s\n",FileName, strerror(errno)); diff --git a/Source/42ipc.c b/Source/42ipc.c index 80bc08f4..220d7fac 100644 --- a/Source/42ipc.c +++ b/Source/42ipc.c @@ -52,20 +52,20 @@ void InitInterProcessComm(void) for(Iipc=0;IipcMode = DecodeString(response); - fscanf(infile,"\"%[^\"]\" %[^\n] %[\n]",FileName,junk,&newline); - fscanf(infile,"%s %[^\n] %[\n]",response,junk,&newline); + fscanf(infile,"\"%79[^\"]\" %119[^\n] %1[\n]",FileName,junk,&newline); + fscanf(infile,"%119s %119[^\n] %1[\n]",response,junk,&newline); I->SocketRole = DecodeString(response); - fscanf(infile,"%s %ld %[^\n] %[\n]",I->HostName,&I->Port,junk,&newline); - fscanf(infile,"%s %[^\n] %[\n]",response,junk,&newline); + fscanf(infile,"%39s %ld %119[^\n] %1[\n]",I->HostName,&I->Port,junk,&newline); + fscanf(infile,"%119s %119[^\n] %1[\n]",response,junk,&newline); I->AllowBlocking = DecodeString(response); - fscanf(infile,"%s %[^\n] %[\n]",response,junk,&newline); + fscanf(infile,"%119s %119[^\n] %1[\n]",response,junk,&newline); I->EchoEnabled = DecodeString(response); fscanf(infile,"%ld %[^\n] %[\n]",&I->Nprefix,junk,&newline); I->Prefix = (char **) calloc(I->Nprefix,sizeof(char *)); for(Ipx=0;IpxNprefix;Ipx++) { - fscanf(infile,"\"%[^\"]\" %[^\n] %[\n]",Prefix,junk,&newline); + fscanf(infile,"\"%79[^\"]\" %119[^\n] %1[\n]",Prefix,junk,&newline); I->Prefix[Ipx] = (char *) calloc(strlen(Prefix)+1,sizeof(char)); strcpy(I->Prefix[Ipx],Prefix); } From 618e0618f212192e5050ec4de3d7ded0d1e0b05c Mon Sep 17 00:00:00 2001 From: nurdmuny Date: Tue, 9 Jun 2026 07:13:53 -0700 Subject: [PATCH 2/2] World/{Mercator,DEM,Albedo}ToCube: same OpenFile fix as iokit.c::FileOpen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scj-hunt's per-target catalog flagged these three OpenFile copies at score 10.0 — same strcpy+strcat-into-FileName[80] pattern as iokit.c::FileOpen (FileName[1024]). Even smaller buffer makes the overflow easier to trigger from a long Path or File argument. All three converted to snprintf with the same truncation-detection shape used in iokit.c. The three tools (Mercator/DEM/Albedo cube converters) share the same OpenFile helper because of copy-paste; ideally they would refactor to call the canonical iokit.c::FileOpen, but that's an architectural change out of scope here. --- World/AlbedoToCube.c | 10 ++++++++-- World/DEMToBumpCube.c | 10 ++++++++-- World/MercatorToCube.c | 12 ++++++++++-- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/World/AlbedoToCube.c b/World/AlbedoToCube.c index 7d10f069..95b1f489 100755 --- a/World/AlbedoToCube.c +++ b/World/AlbedoToCube.c @@ -18,9 +18,15 @@ FILE *OpenFile(char *Path, char *File, char *CtrlCode) { FILE *FilePtr; char FileName[80]; + int Written; - strcpy(FileName,Path); - strcat(FileName,File); + /* snprintf instead of strcpy+strcat — see iokit.c::FileOpen */ + Written = snprintf(FileName,sizeof(FileName),"%s%s",Path,File); + if(Written < 0 || (size_t)Written >= sizeof(FileName)) { + printf("Error: combined path length exceeds %zu bytes: %s%s\n", + sizeof(FileName)-1, Path, File); + exit(1); + } FilePtr=fopen(FileName,CtrlCode); if(FilePtr == NULL) { printf("Error opening %s\n",FileName); diff --git a/World/DEMToBumpCube.c b/World/DEMToBumpCube.c index 01822f94..ca8a81e1 100755 --- a/World/DEMToBumpCube.c +++ b/World/DEMToBumpCube.c @@ -23,9 +23,15 @@ FILE *OpenFile(char *Path, char *File, char *CtrlCode) { FILE *FilePtr; char FileName[80]; + int Written; - strcpy(FileName,Path); - strcat(FileName,File); + /* snprintf instead of strcpy+strcat — see iokit.c::FileOpen */ + Written = snprintf(FileName,sizeof(FileName),"%s%s",Path,File); + if(Written < 0 || (size_t)Written >= sizeof(FileName)) { + printf("Error: combined path length exceeds %zu bytes: %s%s\n", + sizeof(FileName)-1, Path, File); + exit(1); + } FilePtr=fopen(FileName,CtrlCode); if(FilePtr == NULL) { printf("Error opening %s\n",FileName); diff --git a/World/MercatorToCube.c b/World/MercatorToCube.c index ef1a1098..5e9aef10 100755 --- a/World/MercatorToCube.c +++ b/World/MercatorToCube.c @@ -18,9 +18,17 @@ FILE *OpenFile(char *Path, char *File, char *CtrlCode) { FILE *FilePtr; char FileName[80]; + int Written; - strcpy(FileName,Path); - strcat(FileName,File); + /* snprintf instead of strcpy+strcat — see iokit.c::FileOpen for + the equivalent fix on the main entry path. Same bug shape: + attacker-controlled Path + File would overflow FileName[80]. */ + Written = snprintf(FileName,sizeof(FileName),"%s%s",Path,File); + if(Written < 0 || (size_t)Written >= sizeof(FileName)) { + printf("Error: combined path length exceeds %zu bytes: %s%s\n", + sizeof(FileName)-1, Path, File); + exit(1); + } FilePtr=fopen(FileName,CtrlCode); if(FilePtr == NULL) { printf("Error opening %s\n",FileName);