Discussion:
[newlib-cygwin] Cygwin: normalize_win32_path: Avoid buffer underruns
Corinna Vinschen
2018-05-29 16:23:30 UTC
Permalink
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=35998fc2fa6cbb7d761f6d88346246bd3627552b

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

Cygwin: normalize_win32_path: Avoid buffer underruns

Thanks to 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 <***@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;
}

Loading...