Skip to content

Stack buffer overflows in FileOpen and InitInterProcessComm config parsing #191

@nurdymuny

Description

@nurdymuny

Summary

Two memory-safety issues on the config-file ingest paths of 42, both classic unbounded-input-into-fixed-buffer patterns. Either one is exploitable when 42 is run on a hand-crafted or third-party-supplied config file.

1. FileOpen — unbounded strcpy/strcat into 1024-byte stack buffer

Kit/Source/iokit.c:23–36:

FILE *FileOpen(const char *Path, const char *File, const char *CtrlCode)
{
      FILE *FilePtr;
      char FileName[1024];

      strcpy(FileName,Path);
      strcat(FileName,File);
      FilePtr=fopen(FileName,CtrlCode);
      ...
}

If Path + File together exceed 1024 bytes (a long working directory plus a long config file name, easily reachable in CI sweeps), this overflows FileName on the stack. FileOpen is called from every config-file load throughout 42, so any user with control over the working directory or a config filename can trigger it.

2. InitInterProcessComm — five unbounded fscanf conversions

Source/42ipc.c:55–68:

char junk[120], response[120], FileName[80], Prefix[80];
struct IpcType *I;   /* I->HostName is char[40] */

fscanf(infile,"%s %[^\n] %[\n]",response,junk,&newline);
fscanf(infile,"\"%[^\"]\" %[^\n] %[\n]",FileName,junk,&newline);
fscanf(infile,"%s %ld %[^\n] %[\n]",I->HostName,&I->Port,junk,&newline);
fscanf(infile,"\"%[^\"]\" %[^\n] %[\n]",Prefix,junk,&newline);

Every %s / %[^...] here is unboundedfscanf will write past the buffer end if the input field is longer than the buffer. A malicious or accidentally malformed Inp_IPC.txt with:

  • a hostname longer than 39 characters → overflows I->HostName[40]
  • a Mode/SocketRole/etc. response longer than 119 characters → overflows response[120]
  • a filename in quotes longer than 79 characters → overflows FileName[80] or Prefix[80]

This is the canonical CWE-120 / CWE-787 buffer overflow pattern.

Threat model

42 is widely used in NASA Goddard and academic spacecraft research. Users routinely:

  • run 42 on config files received from collaborators (PR-style sharing of spacecraft definitions)
  • run 42 in CI on a corpus of test cases, some of which may be malformed
  • run 42 on auto-generated configs from larger-pipeline tools

Any of these is enough to make an external party effectively control the input file. With typical hardening flags (-fstack-protector-strong) the crash is detected and aborts; without it, classic stack-smashing.

Fix

PR with both fixes attached. Summary:

  • FileOpen: replace strcpy + strcat with snprintf(... "%s%s" ...) + explicit truncation check
  • InitInterProcessComm: add explicit width specifiers (%119s, %39s, %79[^"], etc.) to every fscanf %s / %[^...] so they truncate instead of overrun

Scope note

The same fscanf %s pattern (no width specifier) recurs in Source/42init.c (LoadSC, spacecraft definition parser) and Source/42gl.c (POV and Map config parsers). That's another ~35 sites following the same shape. I've kept this PR focused on the IPC entry-path for ease of review; happy to extend the same fix shape to the rest if you want.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions