diff --git a/src/core/backtrace.cc b/src/core/backtrace.cc index b8962513e9..5f7c2b43f9 100644 --- a/src/core/backtrace.cc +++ b/src/core/backtrace.cc @@ -116,12 +116,16 @@ static SimpleBaseString_sp lu_procname(unw_cursor_t* cursorp) { return SimpleBaseString_O::make(""); } 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); diff --git a/src/core/loadltv.cc b/src/core/loadltv.cc index d137f7e1f8..6392ddf51c 100644 --- a/src/core/loadltv.cc +++ b/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::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 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() { diff --git a/src/core/unixsys.cc b/src/core/unixsys.cc index 849c9acb3d..caab72e69d 100644 --- a/src/core/unixsys.cc +++ b/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) diff --git a/src/gctools/snapshotSaveLoad.cc b/src/gctools/snapshotSaveLoad.cc index 8b32e4c90a..980f26d79a 100644 --- a/src/gctools/snapshotSaveLoad.cc +++ b/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; diff --git a/src/llvmo/llvmoExpose.cc b/src/llvmo/llvmoExpose.cc index b1add90b0a..1e308dd2b2 100644 --- a/src/llvmo/llvmoExpose.cc +++ b/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; diff --git a/src/serveEvent/serveEvent.cc b/src/serveEvent/serveEvent.cc index d7cab9e7da..5d1921b507 100644 --- a/src/serveEvent/serveEvent.cc +++ b/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()); } DOCGROUP(clasp); -CL_DEFUN void serve_event_internal__ll_fd_set(int fd, clasp_ffi::ForeignData_sp fdset) { FD_SET(fd, fdset->data()); } +// 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()); +} 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()); } diff --git a/src/sockets/sockets.cc b/src/sockets/sockets.cc index 43bfdcf1b2..1d87cdfebf 100644 --- a/src/sockets/sockets.cc +++ b/src/sockets/sockets.cc @@ -69,25 +69,36 @@ namespace sockets { #define REINTERPRET_CAST(t, c) reinterpret_cast(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()) { - 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()) { - ok = (size <= strb->arrayTotalSize()); - address = (void*)&(*strb)[0]; // str->addressOfBuffer(); - } else if (core::ComplexVector_T_sp vec = x.asOrNull()) { - 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()) { - 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() {