Re: winsup\cygwin\path.cc issues

[Ken Harris <Ken.Harris@mathworks.com>]

[-- Attachment #1: Type: text/plain, Size: 2393 bytes --]

Hi Marco:
                Sorry for not replying to the original exchange we had. I wasn't subscribed to the list but now I am so it won't happen again (so I'm quoting our exchange below).
                
                I installed and built cygwin1.dll with an added assert in path.cc to identify when the buffer underrun condition I originally described occurs:

$ diff -b ./cygwin-2.10.0-1.src/newlib-cygwin/winsup/cygwin/path.cc.ORIG ./cygwin-2.10.0-1.src/newlib-cygwin/winsup/cygwin/path.cc
2803c2803
<                   ;
---
>                   assert(p >= path);

                Thus, a simple:

                cat '\A../../../B'

                will result in the assert firing:

kharris@ah-kharris /usr/src
$ cat '\A../../../B'
assertion "p >= path" failed: file "../../.././winsup/cygwin/path.cc", line 2803, function: int symlink_info::check(char*, const suffix_info*, fs_info&, path_conv_handle&)
Aborted (core dumped)

Attached is a patch (in addition to the added assert) with what I *think* might really fix the problem. This was where the expected backslash got squashed which allowed symlink_info::check() to go "negative" with its 'p' pointer and look for a backslash in someone else's memory.

                I've applied this "correction" in our MSYS2 code.  I hope to get some flight-time with it soon (long duration, automated processing)  and if it causes unexpected problems, I'll report back on that. Otherwise, I just hope it might be helpful to anyone who might run into similar puzzling circumstances (the puzzle is when the errant 'p' pointer _doesn't_ find a stray backslash in someone else's memory. It segv-s and _that_ was the nasty part of the puzzle).

                Thanks,
                -Ken


On 5/2/2018 3:49 PM, Ken Harris wrote:

    Hi:
                     While originally investigating a sporadic failure in MSYS2, I believe I found that its origin may actually be within Cygwin.

                     Given the following command sequence on cygwin64 in a CMD.EXE command prompt (on Windows 10 x64 if it matters).


       cd C:\Cygwin64\bin
                     echo.exe running \"test\" logging to ../../../my.log



Not clear to me what is the exact command line to replicate

In addition "C:\Cygwin64\bin" is "/bin" so where do you expect
/bin/../../../my.log to be ?

Regards
Marco

[-- Attachment #2: path.cc.patch --]
[-- Type: application/octet-stream, Size: 1745 bytes --]

--- ./cygwin-2.10.0-1.src/newlib-cygwin/winsup/cygwin/path.cc.ORIG	2018-05-03 19:43:00.482472100 -0400
+++ ./cygwin-2.10.0-1.src/newlib-cygwin/winsup/cygwin/path.cc	2018-05-03 20:09:44.035630700 -0400
@@ -1342,6 +1342,7 @@
 int
 normalize_win32_path (const char *src, char *dst, char *&tail)
 {
+  int drvprefixlen = 0;
   const char *src_start = src;
   bool beg_src_slash = isdirsep (src[0]);
 
@@ -1385,9 +1386,10 @@
 	*tail++ = cyg_toupper (*src++);
       else if (*src != '/')
 	{
-	  if (beg_src_slash)
-	    tail += cygheap->cwd.get_drive (dst);
-	  else if (!cygheap->cwd.get (dst, 0))
+	  if (beg_src_slash) {
+	    drvprefixlen = cygheap->cwd.get_drive (dst);
+	    tail += drvprefixlen;
+	  } else if (!cygheap->cwd.get (dst, 0))
 	    return get_errno ();
 	  else
 	    {
@@ -1423,10 +1425,10 @@
 	  else
 	    {
 	      /* Back up over /, but not if it's the first one.  */
-	      if (tail > dst + 1)
+	      if (tail > dst + 1 + drvprefixlen)
 		tail--;
 	      /* Now back up to the next /.  */
-	      while (tail > dst + 1 && tail[-1] != '\\' && tail[-2] != ':')
+	      while (tail > dst + 1 + drvprefixlen && tail[-1] != '\\' && tail[-2] != ':')
 		tail--;
 	      src += 2;
 	      /* Skip /'s to the next path component. */
@@ -1446,7 +1448,7 @@
       if ((tail - dst) >= NT_MAX_PATH)
 	return ENAMETOOLONG;
     }
-  if (tail > dst + 1 && tail[-1] == '.' && tail[-2] == '\\')
+  if (tail > dst + 1 + drvprefixlen && tail[-1] == '.' && tail[-2] == '\\')
     tail--;
   *tail = '\0';
   debug_printf ("%s = normalize_win32_path (%s)", dst, src_start);
@@ -2800,7 +2802,7 @@
 	      if (*p != '.' && *p != ' ')
 		{
 		  while (*--p != '\\')
-		    ;
+		    assert(p >= path);
 		  if (*++p != ' ')
 		    p = NULL;
 		}

[-- Attachment #3: Type: text/plain, Size: 219 bytes --]


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple