public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH] Cygwin: spawn: Fix segfalt when too many command line args are specified.
@ 2023-08-28  9:20 Takashi Yano
  0 siblings, 0 replies; only message in thread
From: Takashi Yano @ 2023-08-28  9:20 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Takashi Yano, Ed Morton

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


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-08-28  9:20 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-28  9:20 [PATCH] Cygwin: spawn: Fix segfalt when too many command line args are specified Takashi Yano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).