Security hardening: bound FASL sizes, refuse unsafe socket buffers, shell-quote save-lisp paths (S2–S10 except S1/S6)

src/core/backtrace.cc

@@ -116,12 +116,16 @@ static SimpleBaseString_sp lu_procname(unw_cursor_t* cursorp) {
       return SimpleBaseString_O::make("<unknown libunwind error>");
     }
     if (nbytes >= 4096) {
-      // Too long. Give up, putting an ellipsis on the end
-      char fnamedot[nbytes + 3];
-      fnamedot[0] = '\0';
-      strcat(fnamedot, fname);
-      strcat(fnamedot, "...");
-      return SimpleBaseString_O::make(fnamedot);
+      // Too long. libunwind returned UNW_ENOMEM, which means it filled (up to)
+      // nbytes bytes and may NOT have written a terminating NUL. The previous
+      // code did strcat(fnamedot, fname), which assumes fname is NUL-terminated
+      // -- without that guarantee it reads off the end of the buffer into the
+      // stack until it finds a NUL and writes that past fnamedot. Bound the
+      // read with strnlen and build the result without strcat.
+      size_t len = strnlen(fname, nbytes);
+      std::string s(fname, len);
+      s.append("...");
+      return SimpleBaseString_O::make(s);
     } else
       nbytes <<= 1;
   } while (true);

src/core/loadltv.cc

@@ -571,8 +571,17 @@ struct loadltv {
     for (size_t i = 0; i < rank; ++i) {
       uint16_t dim = read_u16();
       dims << clasp_make_fixnum(dim);
-      total *= dim;
+      // Overflow-checked: with rank up to 255 and dim up to 0xFFFF, total can
+      // overflow size_t on a malformed FASL, then be passed to make_vector /
+      // make_mdarray as a wrapped-around small value -- mismatch with the per-
+      // element fill loop would read/write past the allocated buffer.
+      if (__builtin_mul_overflow(total, (size_t)dim, &total))
+        SIMPLE_ERROR("Invalid FASL: op_array dimension product overflows size_t");
     }
+    // Defense in depth against absurd FASL-supplied sizes.
+    constexpr size_t MAX_ARRAY_TOTAL = (size_t)1 << 30; // 1G elements
+    if (total > MAX_ARRAY_TOTAL)
+      SIMPLE_ERROR("Invalid FASL: op_array total size {} exceeds limit {}", total, MAX_ARRAY_TOTAL);
     Array_sp arr =
         (rank == 1)
             // very unsure about the cast, but this is an ambiguous ?: otherwise
@@ -642,10 +651,23 @@ struct loadltv {
   void op_bignum() {
     size_t index = next_index();
     int64_t ssize = read_s64();
-    mp_limb_t limbs[std::abs(ssize)];
-    for (size_t i = std::abs(ssize); i > 0; --i)
+    // ssize carries sign; std::abs(ssize) is the limb count. Three hazards in the
+    // original code, all triggerable by one byte of a malformed FASL:
+    //   (1) std::abs(INT64_MIN) is undefined behavior.
+    //   (2) A stack VLA sized straight from file input can overflow the stack
+    //       (or be smashed) on any large value.
+    //   (3) No upper bound -- a malformed FASL can ask for tens of GB.
+    // Reject INT64_MIN, cap at a sane limit, and heap-allocate via std::vector.
+    if (ssize == std::numeric_limits<int64_t>::min())
+      SIMPLE_ERROR("Invalid FASL: op_bignum size INT64_MIN");
+    constexpr uint64_t MAX_LIMBS = (uint64_t)1 << 20; // ~8 MB bignum, very generous
+    uint64_t size = (uint64_t)std::abs(ssize);
+    if (size > MAX_LIMBS)
+      SIMPLE_ERROR("Invalid FASL: op_bignum size {} exceeds limit {}", size, MAX_LIMBS);
+    std::vector<mp_limb_t> limbs(size);
+    for (size_t i = size; i > 0; --i)
       limbs[i - 1] = read_u64();
-    set_ltv(bignum_result(ssize, limbs), index);
+    set_ltv(bignum_result(ssize, limbs.data()), index);
   }
 
   void op_binary16() {

src/core/unixsys.cc

@@ -150,8 +150,10 @@ static void from_list_to_execve_argument(T_sp l, char*** environp) {
     cl_index l = ss.size();
     environ[j] = (char*)malloc(l + 1);
     memcpy(environ[j], ss.c_str(), l);
+    environ[j][l] = 0; // terminate THIS string before advancing -- the previous
+                       // order (`j++; environ[j][l]=0;`) wrote into the next,
+                       // uninitialized slot's pointer = wild write + missing NUL.
     j++;
-    environ[j][l] = 0;
   }
   environ[j] = 0;
   if (environp)

src/gctools/snapshotSaveLoad.cc

@@ -108,6 +108,27 @@ const char* pointer_pool(void* pointer) {
     gctools::truly_abort();                                                                                                        \
   }
 
+// Wrap a string as a single shell-safe argument: single-quote it and escape
+// any embedded single quote as '\''. The previous `system("..." + filename)`
+// and `popen("nm \"" << filename << "\"", "r")` patterns let any shell
+// metacharacter in `filename`/`_FileName`/`_LibDir` inject commands (e.g.
+// save-lisp-and-die :executable t :file "foo; rm -rf /"). Wrapping every user-
+// controlled value with this before interpolation makes the shell treat it as
+// one literal argument, no matter what bytes it contains.
+static std::string shell_quote(const std::string& s) {
+  std::string out;
+  out.reserve(s.size() + 2);
+  out.push_back('\'');
+  for (char c : s) {
+    if (c == '\'')
+      out.append("'\\''");
+    else
+      out.push_back(c);
+  }
+  out.push_back('\'');
+  return out;
+}
+
 /*! Build a LibraryLookup by running 'nm' on one of our loaded libraries or executable.
  *  For dynamic libraries on linux (contain .so in filename) use --dynamic because regular symbols are often stripped
  *  Look for the first 'T' symbol and dlsym it to find out where the library is loaded in memory.
@@ -135,15 +156,15 @@ bool loadLibrarySymbolLookup(const std::string& filename, LibraryLookup& library
   std::string dynamic = "";
   if (filename.find(".so") != std::string::npos)
     dynamic = "--dynamic ";
-  nm_cmd << NM_BINARY << " " << dynamic << "-p --defined-only --no-sort \"" << filename << "\"";
+  nm_cmd << NM_BINARY << " " << dynamic << "-p --defined-only --no-sort " << shell_quote(filename);
 #elif defined(_TARGET_OS_DARWIN)
   gctools::clasp_ptr_t start;
   gctools::clasp_ptr_t end;
   core::executableTextSectionRange(start, end);
-  nm_cmd << NM_BINARY << " -p --defined-only \"" << filename << "\"";
+  nm_cmd << NM_BINARY << " -p --defined-only " << shell_quote(filename);
 #else
 #error "Handle other operating systems - how is main found using dlsym and in the output of nm"
-  nm_cmd << NM_BINARY << " -p --defined-only \"" << filename << "\"";
+  nm_cmd << NM_BINARY << " -p --defined-only " << shell_quote(filename);
 #endif
   if (fout)
     fprintf(fout, "# Symbols obtained by filtering: %s\n", nm_cmd.str().c_str());
@@ -1854,9 +1875,12 @@ void* snapshot_save_impl(void* data) {
         mangled_name.begin(), mangled_name.end(), [](unsigned char c) { return !std::isalnum(c); }, '_');
 
     std::cout << "Creating binary object from snapshot..." << std::endl << std::flush;
+    // shell_quote user-controlled paths to prevent command injection through
+    // the save-lisp-and-die :file argument. mangled_name is already restricted
+    // to [A-Za-z0-9_] above, so it's safe unquoted.
     cmd = OBJCOPY_BINARY " --input-target binary --output-target elf64-x86-64"
                          " --binary-architecture i386 " +
-          filename + " " + obj_filename + " --redefine-sym _binary_" + mangled_name +
+          shell_quote(filename) + " " + shell_quote(obj_filename) + " --redefine-sym _binary_" + mangled_name +
           "_start=" CXX_MACRO_STRING(SNAPSHOT_START) " --redefine-sym _binary_" + mangled_name +
           "_end=" CXX_MACRO_STRING(SNAPSHOT_END) " --redefine-sym _binary_" + mangled_name +
           "_size=" CXX_MACRO_STRING(SNAPSHOT_SIZE);
@@ -1865,8 +1889,8 @@ void* snapshot_save_impl(void* data) {
       return NULL;
     }
 
-    cmd = CXX_BINARY " " BUILD_LINKFLAGS " -L" + snapshot_data->_LibDir + " -o" + snapshot_data->_FileName + " " + obj_filename +
-          " -Wl,-whole-archive -liclasp"
+    cmd = CXX_BINARY " " BUILD_LINKFLAGS " -L" + shell_quote(snapshot_data->_LibDir) + " -o" + shell_quote(snapshot_data->_FileName) +
+          " " + shell_quote(obj_filename) + " -Wl,-whole-archive -liclasp"
 #ifndef CLASP_STATIC_LINKING
           " -Wl,-no-whole-archive"
 #endif
@@ -1877,9 +1901,9 @@ void* snapshot_save_impl(void* data) {
           BUILD_LIB;
 #endif
 #ifdef _TARGET_OS_DARWIN
-    cmd = CXX_BINARY " " BUILD_LINKFLAGS " -o" + snapshot_data->_FileName +
-          " -sectcreate " SNAPSHOT_SEGMENT " " SNAPSHOT_SECTION " " + filename + " -Wl,-force_load," + snapshot_data->_LibDir +
-          "/libiclasp.a -lclasp " BUILD_LIB;
+    cmd = CXX_BINARY " " BUILD_LINKFLAGS " -o" + shell_quote(snapshot_data->_FileName) +
+          " -sectcreate " SNAPSHOT_SEGMENT " " SNAPSHOT_SECTION " " + shell_quote(filename) +
+          " -Wl,-force_load," + shell_quote(snapshot_data->_LibDir + "/libiclasp.a") + " -lclasp " BUILD_LIB;
 #endif
 
     std::cout << "Link command:" << std::endl << std::flush;

src/llvmo/llvmoExpose.cc

@@ -255,7 +255,18 @@ const char* my_LLVMSymbolLookupCallback(void* DisInfo, uint64_t ReferenceValue,
       ss << "[" << dbg_safe_repr(*(void**)ReferenceValue) << "]";
     }
     ss << "}";
-    strcpy(global_LLVMSymbolLookupCallbackBuffer, ss.str().c_str());
+    {
+      // Bounded copy: ss may exceed CALLBACK_BUFFER_SIZE (e.g. when
+      // dbg_safe_repr emits a long object printout). strcpy with no length
+      // check would overflow the fixed buffer. Truncate -- this is a JIT
+      // debug/diagnostic path, so a clipped name is acceptable.
+      const std::string s = ss.str();
+      size_t n = s.size() < (size_t)(CALLBACK_BUFFER_SIZE - 1)
+                     ? s.size()
+                     : (size_t)(CALLBACK_BUFFER_SIZE - 1);
+      memcpy(global_LLVMSymbolLookupCallbackBuffer, s.data(), n);
+      global_LLVMSymbolLookupCallbackBuffer[n] = '\0';
+    }
     *ReferenceName = global_LLVMSymbolLookupCallbackBuffer;
     //    printf("%s:%d:%s Returning symbol-table result |%s|\n", __FILE__, __LINE__, __FUNCTION__, *ReferenceName);
     return *ReferenceName;
@@ -268,7 +279,18 @@ const char* my_LLVMSymbolLookupCallback(void* DisInfo, uint64_t ReferenceValue,
     if (ReferenceValue != (uintptr_t)data.dli_saddr) {
       ss << "+" << (ReferenceValue - (uintptr_t)data.dli_saddr);
     }
-    strcpy(global_LLVMSymbolLookupCallbackBuffer, ss.str().c_str());
+    {
+      // Bounded copy: ss may exceed CALLBACK_BUFFER_SIZE (e.g. when
+      // dbg_safe_repr emits a long object printout). strcpy with no length
+      // check would overflow the fixed buffer. Truncate -- this is a JIT
+      // debug/diagnostic path, so a clipped name is acceptable.
+      const std::string s = ss.str();
+      size_t n = s.size() < (size_t)(CALLBACK_BUFFER_SIZE - 1)
+                     ? s.size()
+                     : (size_t)(CALLBACK_BUFFER_SIZE - 1);
+      memcpy(global_LLVMSymbolLookupCallbackBuffer, s.data(), n);
+      global_LLVMSymbolLookupCallbackBuffer[n] = '\0';
+    }
     *ReferenceName = global_LLVMSymbolLookupCallbackBuffer;
     //    printf("%s:%d:%s Returning dladdr result |%s|\n", __FILE__, __LINE__, __FUNCTION__, *ReferenceName);
     return *ReferenceName;

src/serveEvent/serveEvent.cc

@@ -42,10 +42,20 @@ DOCGROUP(clasp);
 CL_DEFUN void serve_event_internal__ll_fd_zero(clasp_ffi::ForeignData_sp fdset) { FD_ZERO(fdset->data<fd_set*>()); }
 
 DOCGROUP(clasp);
-CL_DEFUN void serve_event_internal__ll_fd_set(int fd, clasp_ffi::ForeignData_sp fdset) { FD_SET(fd, fdset->data<fd_set*>()); }
+// Range-check fd: FD_SET/FD_ISSET write/read out of bounds of the fd_set bit
+// array for fd outside [0, FD_SETSIZE).
+static inline void serve_event_check_fd(int fd) {
+  if (fd < 0 || fd >= FD_SETSIZE)
+    SIMPLE_ERROR("File descriptor {} out of range for fd_set [0..{})", fd, (int)FD_SETSIZE);
+}
+CL_DEFUN void serve_event_internal__ll_fd_set(int fd, clasp_ffi::ForeignData_sp fdset) {
+  serve_event_check_fd(fd);
+  FD_SET(fd, fdset->data<fd_set*>());
+}
 
 DOCGROUP(clasp);
 CL_DEFUN int serve_event_internal__ll_fd_isset(int fd, clasp_ffi::ForeignData_sp fdset) {
+  serve_event_check_fd(fd);
   return FD_ISSET(fd, fdset->data<fd_set*>());
 }
 

src/sockets/sockets.cc

@@ -69,25 +69,36 @@ namespace sockets {
 
 #define REINTERPRET_CAST(t, c) reinterpret_cast<t>(c)
 
-static void* safe_buffer_pointer(core::T_sp x, uint size) {
+static void* safe_buffer_pointer(core::T_sp x, int size) {
+  // Reject negative sizes here: callers pass `int length` from Lisp, and a
+  // signed -> unsigned conversion would underflow into multi-GB. The syscall
+  // layer would then receive/send gigabytes into/from a small Lisp buffer.
+  if (size < 0)
+    SIMPLE_ERROR("Negative socket buffer size: {}", size);
+  size_t usize = (size_t)size;
   bool ok = false;
   void* address;
   if (core::Str8Ns_sp str = x.asOrNull<core::Str8Ns_O>()) {
-    ok = (size <= str->arrayTotalSize());
-    address = (void*)&(*str)[0]; // str->addressOfBuffer();
+    ok = (usize <= str->arrayTotalSize());
+    address = (void*)&(*str)[0];
   } else if (core::SimpleBaseString_sp strb = x.asOrNull<core::SimpleBaseString_O>()) {
-    ok = (size <= strb->arrayTotalSize());
-    address = (void*)&(*strb)[0]; // str->addressOfBuffer();
-  } else if (core::ComplexVector_T_sp vec = x.asOrNull<core::ComplexVector_T_O>()) {
-    int divisor = vec->elementSizeInBytes();
-    size = (size + divisor - 1) / divisor;
-    ok = (size <= vec->arrayTotalSize());
-    address = &(*vec)[0];
+    ok = (usize <= strb->arrayTotalSize());
+    address = (void*)&(*strb)[0];
   } else if (core::SimpleVector_byte8_t_sp svb8 = x.asOrNull<core::SimpleVector_byte8_t_O>()) {
-    ok = (size <= svb8->arrayTotalSize());
+    ok = (usize <= svb8->arrayTotalSize());
     address = svb8->rowMajorAddressOfElement_(0);
   } else {
-    SIMPLE_ERROR("Add support for buffer {}", _rep_(x));
+    // Refuse anything else -- in particular (vector t): the previous code
+    // accepted it via a ComplexVector_T_O branch, but writing raw socket bytes
+    // over a vector of boxed Lisp pointers corrupts the GC heap (recvfrom into
+    // such a buffer would scribble network bytes over tagged pointer values).
+    // Its (size+divisor-1)/divisor capacity check also wrapped on a negative
+    // `length` cast to uint, bypassing the bounds check entirely. High-level
+    // socket wrappers use strings or (simple-array (unsigned-byte 8)).
+    SIMPLE_ERROR("Refusing socket buffer of unsupported type {}; pass a string "
+                 "or (simple-array (unsigned-byte 8)) instead -- writing raw "
+                 "network bytes into a vector of boxed values would corrupt the heap",
+                 _rep_(x));
   }
   if (!ok) {
     SIMPLE_ERROR("Lisp object does not have enough space to be a valid socket buffer: {}", _rep_(x));
@@ -725,13 +736,28 @@ DOCGROUP(clasp);
 CL_DEFUN void sockets_internal__fdset_zero(core::Pointer_sp p) { FD_ZERO((fd_set*)p->ptr()); }
 
 DOCGROUP(clasp);
-CL_DEFUN void sockets_internal__fdset_set(gc::Fixnum fd, core::Pointer_sp fdset) { FD_SET(fd, (fd_set*)fdset->ptr()); }
+// Range-check fd against FD_SETSIZE: FD_SET/FD_CLR/FD_ISSET index into a
+// fixed-size bit array and write/read out of bounds for fd outside [0,FD_SETSIZE).
+static inline void check_fdset_fd(gc::Fixnum fd) {
+  if (fd < 0 || fd >= FD_SETSIZE)
+    SIMPLE_ERROR("File descriptor {} out of range for fd_set [0..{})", fd, (int)FD_SETSIZE);
+}
+CL_DEFUN void sockets_internal__fdset_set(gc::Fixnum fd, core::Pointer_sp fdset) {
+  check_fdset_fd(fd);
+  FD_SET(fd, (fd_set*)fdset->ptr());
+}
 
 DOCGROUP(clasp);
-CL_DEFUN void sockets_internal__fdset_clr(gc::Fixnum fd, core::Pointer_sp fdset) { FD_CLR(fd, (fd_set*)fdset->ptr()); }
+CL_DEFUN void sockets_internal__fdset_clr(gc::Fixnum fd, core::Pointer_sp fdset) {
+  check_fdset_fd(fd);
+  FD_CLR(fd, (fd_set*)fdset->ptr());
+}
 
 DOCGROUP(clasp);
-CL_DEFUN bool sockets_internal__fdset_isset(gc::Fixnum fd, core::Pointer_sp fdset) { return FD_ISSET(fd, (fd_set*)fdset->ptr()); }
+CL_DEFUN bool sockets_internal__fdset_isset(gc::Fixnum fd, core::Pointer_sp fdset) {
+  check_fdset_fd(fd);
+  return FD_ISSET(fd, (fd_set*)fdset->ptr());
+}
 
 DOCGROUP(clasp);
 CL_DEFUN core::T_sp sockets_internal__get_host_name() {