[newlib-cygwin] Cygwin: normalize_win32_path: Avoid buffer underruns

Corinna Vinschen corinna@sourceware.org
Tue May 29 16:23:00 GMT 2018


https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=35998fc2fa6cbb7d761f6d88346246bd3627552b

commit 35998fc2fa6cbb7d761f6d88346246bd3627552b
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Tue May 29 18:11:42 2018 +0200

    Cygwin: normalize_win32_path: Avoid buffer underruns
    
    Thanks to Ken Harris <Ken.Harris@mathworks.com> for the diagnosis.
    
    When backing up tail to handle a "..", the code only checked that
    it didn't underrun the destination buffer while removing path
    components.  It did *not* take into account that the first backslash
    in the path had to be kept intact.  Example path to trigger the
    problem: "C:\A..\..\..\B'
    
    Fix this by moving the dst pointer to the first backslash so subsequent
    tests cannot underrun this position.  Also make sure that we always
    *have* a backslash.
    
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/cygwin/path.cc | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index 94f4e88..3c4dd30 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -1336,6 +1336,7 @@ int
 normalize_win32_path (const char *src, char *dst, char *&tail)
 {
   const char *src_start = src;
+  const char *dst_start = dst;
   bool beg_src_slash = isdirsep (src[0]);
 
   tail = dst;
@@ -1370,27 +1371,43 @@ normalize_win32_path (const char *src, char *dst, char *&tail)
 	      src += 2;
 	    }
 	}
+      dst = tail;
+      /* If backslash is missing in src, add one. */
+      if (!isdirsep (src[0]))
+	*tail++ = '\\';
     }
-  if (tail == dst)
+  if (tail == dst_start)
     {
       if (isdrive (src))
-	/* Always convert drive letter to uppercase for case sensitivity. */
-	*tail++ = cyg_toupper (*src++);
+	{
+	  /* Always convert drive letter to uppercase for case sensitivity. */
+	  *tail++ = cyg_toupper (*src++);
+	  *tail++ = *src++;
+	  dst = tail;
+	  /* If backslash is missing in src, add one. */
+	  if (!isdirsep (src[0]))
+	    *tail++ = '\\';
+	}
       else if (*src != '/')
 	{
 	  if (beg_src_slash)
-	    tail += cygheap->cwd.get_drive (dst);
-	  else if (!cygheap->cwd.get (dst, 0))
-	    return get_errno ();
-	  else
+	    dst = (tail += cygheap->cwd.get_drive (dst));
+	  else if (cygheap->cwd.get (dst, 0))
 	    {
 	      tail = strchr (tail, '\0');
 	      if (tail[-1] != '\\')
 		*tail++ = '\\';
+	      dst = tail - 1;
 	    }
+	  else
+	    return get_errno ();
 	}
     }
 
+  /* At this point dst points to the first backslash, even if it only gets
+     written in the first iteration of the following loop.  Backing up to
+     handle ".." components can not underrun that border (thus avoiding
+     subsequent buffer underruns with fatal results). */
   while (*src)
     {
       /* Strip duplicate /'s.  */
@@ -1442,7 +1459,7 @@ normalize_win32_path (const char *src, char *dst, char *&tail)
   if (tail > dst + 1 && tail[-1] == '.' && tail[-2] == '\\')
     tail--;
   *tail = '\0';
-  debug_printf ("%s = normalize_win32_path (%s)", dst, src_start);
+  debug_printf ("%s = normalize_win32_path (%s)", dst_start, src_start);
   return 0;
 }



More information about the Cygwin-cvs mailing list