[PATCH] Cygwin: spawn: Fix segfalt when too many command line args are specified.

Takashi Yano takashi.yano@nifty.ne.jp
Mon Aug 28 09:20:12 GMT 2023


Previously, the number of command line args was not checked for
cygwin process. Due to this, segmentation fault was caused if too
many command line args are specified.
https://cygwin.com/pipermail/cygwin/2023-August/254333.html

Since char *argv[argc + 1] is placed on the stack in dll_crt0_1(),
STATUS_STACK_OVERFLOW occurs if the stack does not have enough
space.

With this patch, the total length of the arguments is restricted to
1/4 of total stack size for the process, and spawnve() returns E2BIG if
the size exceeds the limit.

Reported-by: Ed Morton <mortoneccc@comcast.net>
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
---
 winsup/cygwin/release/3.4.9 |  3 +++
 winsup/cygwin/spawn.cc      | 41 ++++++++++++++++++++++++++++++++++++-
 winsup/cygwin/sysconf.cc    |  9 +++++++-
 3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/release/3.4.9 b/winsup/cygwin/release/3.4.9
index 2f2da9e13..53c4e5fc8 100644
--- a/winsup/cygwin/release/3.4.9
+++ b/winsup/cygwin/release/3.4.9
@@ -8,3 +8,6 @@ Bug Fixes
 - For the time being, disable creating special files using mknod/mkfifo
   on NFS.
   Addresses: https://cygwin.com/pipermail/cygwin/2023-August/254266.html
+
+- Fix segfault when too many command line args are specified.
+  Addresses: https://cygwin.com/pipermail/cygwin/2023-August/254333.html
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index c16fe269a..b38e4de27 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -284,6 +284,29 @@ extern "C" void __posix_spawn_sem_release (void *sem, int error);
 
 extern DWORD mutex_timeout; /* defined in fhandler_termios.cc */
 
+static size_t
+get_stack_size (const WCHAR *filename)
+{
+  HANDLE h;
+  h = CreateFileW (filename, GENERIC_READ, FILE_SHARE_READ,
+		   NULL, OPEN_EXISTING, 0, NULL);
+  char buf[1024];
+  DWORD n;
+  ReadFile (h, buf, sizeof (buf), &n, 0);
+  CloseHandle (h);
+  IMAGE_NT_HEADERS32 *p = (IMAGE_NT_HEADERS32 *) memmem (buf, n, "PE\0\0", 4);
+  if (!p)
+    return 0;
+  if ((char *) &p->OptionalHeader.SizeOfStackCommit > buf + n)
+    return 0; /* buf[] is not enough */
+  if (p->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC)
+    return p->OptionalHeader.SizeOfStackReserve;
+  IMAGE_NT_HEADERS64 *p64 = (IMAGE_NT_HEADERS64 *) p;
+  if ((char *) &p64->OptionalHeader.SizeOfStackCommit > buf + n)
+    return 0; /* buf[] is not enough */
+  return p64->OptionalHeader.SizeOfStackReserve;
+}
+
 int
 child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 			  const char *const envp[], int mode,
@@ -351,8 +374,9 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	 We need to quote any argument that has whitespace or embedded "'s.  */
 
       int ac;
+      size_t arg_len = 0;
       for (ac = 0; argv[ac]; ac++)
-	/* nothing */;
+	arg_len += strlen (argv[ac]) + 1;
 
       int err;
       const char *ext;
@@ -621,6 +645,21 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	    }
 	}
 
+      if (iscygwin ())
+	{
+	  size_t child_stack_size = get_stack_size (runpath);
+	  /* char *argv[] will be placed on the stack in dll_crt0_1(), so
+	     restrict total argument length to 1/4 of total stack size. */
+	  bool too_many_args = child_stack_size ?
+	    arg_len > child_stack_size / 4 : ac >= MAXWINCMDLEN;
+	  if (too_many_args)
+	    {
+	      set_errno (E2BIG);
+	      res = -1;
+	      __leave;
+	    }
+	}
+
       bool no_pcon = mode != _P_OVERLAY && mode != _P_WAIT;
       term_spawn_worker.setup (iscygwin (), handle (fileno_stdin, false),
 			       runpath, no_pcon, reset_sendsig, envblock);
diff --git a/winsup/cygwin/sysconf.cc b/winsup/cygwin/sysconf.cc
index 2db92e4de..6cb2aecd0 100644
--- a/winsup/cygwin/sysconf.cc
+++ b/winsup/cygwin/sysconf.cc
@@ -21,6 +21,13 @@ details. */
 #include "cpuid.h"
 #include "clock.h"
 
+#define DEFAULT_STACKGUARD (wincap.def_guard_page_size() + wincap.page_size ())
+static long
+get_arg_max (int in)
+{
+  return (long) (get_rlimit_stack () + DEFAULT_STACKGUARD) / 4;
+}
+
 static long
 get_page_size (int in)
 {
@@ -485,7 +492,7 @@ static struct
     };
 } sca[] =
 {
-  {cons, {c:ARG_MAX}},			/*   0, _SC_ARG_MAX */
+  {func, {f:get_arg_max}},		/*   0, _SC_ARG_MAX */
   {cons, {c:CHILD_MAX}},		/*   1, _SC_CHILD_MAX */
   {cons, {c:CLOCKS_PER_SEC}},		/*   2, _SC_CLK_TCK */
   {cons, {c:NGROUPS_MAX}},		/*   3, _SC_NGROUPS_MAX */
-- 
2.39.0



More information about the Cygwin-patches mailing list