From 35998fc2fa6cbb7d761f6d88346246bd3627552b Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Tue, 29 May 2018 18:11:42 +0200 Subject: [PATCH] Cygwin: normalize_win32_path: Avoid buffer underruns Thanks to Ken Harris 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 --- 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 94f4e887c..3c4dd3077 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; }