From baa45079466eda1f5636a6d13f3a60c2c00fdcd3 Mon Sep 17 00:00:00 2001
From: Steve Dower <steve.dower@microsoft.com>
Date: Mon, 5 Mar 2018 14:26:28 -0800
Subject: [PATCH] [3.6] bpo-33001: Prevent buffer overrun in os.symlink
 (GH-5989) (GH-5990)

---
 Lib/test/test_os.py                                | 35 ++++++++++++
 .../2018-03-05-10-09-51.bpo-33001.elj4Aa.rst       |  1 +
 Modules/posixmodule.c                              | 66 +++++++++++++---------
 3 files changed, 74 insertions(+), 28 deletions(-)
 create mode 100644 Misc/NEWS.d/next/Security/2018-03-05-10-09-51.bpo-33001.elj4Aa.rst

diff --git a/Misc/NEWS.d/next/Security/2018-03-05-10-09-51.bpo-33001.elj4Aa.rst b/Misc/NEWS.d/next/Security/2018-03-05-10-09-51.bpo-33001.elj4Aa.rst
new file mode 100644
index 000000000000..2acbac9e1af6
--- /dev/null
+++ b/Misc/NEWS.d/next/Security/2018-03-05-10-09-51.bpo-33001.elj4Aa.rst
@@ -0,0 +1 @@
+Minimal fix to prevent buffer overrun in os.symlink on Windows
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 0837a4a4991e..39ba030b5191 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -7241,7 +7241,7 @@ win_readlink(PyObject *self, PyObject *args, PyObject *kwargs)
 #if defined(MS_WINDOWS)
 
 /* Grab CreateSymbolicLinkW dynamically from kernel32 */
-static DWORD (CALLBACK *Py_CreateSymbolicLinkW)(LPWSTR, LPWSTR, DWORD) = NULL;
+static BOOLEAN (CALLBACK *Py_CreateSymbolicLinkW)(LPCWSTR, LPCWSTR, DWORD) = NULL;
 static DWORD (CALLBACK *Py_CreateSymbolicLinkA)(LPSTR, LPSTR, DWORD) = NULL;
 
 static int
@@ -7259,18 +7259,24 @@ check_CreateSymbolicLink(void)
     return (Py_CreateSymbolicLinkW && Py_CreateSymbolicLinkA);
 }
 
-/* Remove the last portion of the path */
-static void
+/* Remove the last portion of the path - return 0 on success */
+static int
 _dirnameW(WCHAR *path)
 {
     WCHAR *ptr;
 
+    size_t length = wcsnlen_s(path, MAX_PATH);
+    if (length == MAX_PATH) {
+        return -1;
+    }
     /* walk the path from the end until a backslash is encountered */
-    for(ptr = path + wcslen(path); ptr != path; ptr--) {
-        if (*ptr == L'\\' || *ptr == L'/')
+    for(ptr = path + length; ptr != path; ptr--) {
+        if (*ptr == L'\\' || *ptr == L'/'){
             break;
+   }
     }
     *ptr = 0;
+    return 0;
 }
 
 /* Remove the last portion of the path */
@@ -7299,29 +7305,26 @@ _is_absW(const WCHAR *path)
 static int
 _is_absA(const char *path)
 {
-    return path[0] == '\\' || path[0] == '/' || path[1] == ':';
-
+    return path[0] == L'\\' || path[0] == L'/' ||
+        (path[0] && path[1] == L':');
 }
 
-/* join root and rest with a backslash */
-static void
+/* join root and rest with a backslash - return 0 on success */
+static int
 _joinW(WCHAR *dest_path, const WCHAR *root, const WCHAR *rest)
 {
-    size_t root_len;
-
     if (_is_absW(rest)) {
-        wcscpy(dest_path, rest);
-        return;
+        return wcscpy_s(dest_path, MAX_PATH, rest);
     }
 
-    root_len = wcslen(root);
-
-    wcscpy(dest_path, root);
-    if(root_len) {
-        dest_path[root_len] = L'\\';
-        root_len++;
+    if (wcscpy_s(dest_path, MAX_PATH, root)) {
+        return -1;
     }
-    wcscpy(dest_path+root_len, rest);
+
+    if (dest_path[0] && wcscat_s(dest_path, MAX_PATH, L"\\")) {
+        return -1;
+     }
+    return wcscat_s(dest_path, MAX_PATH, rest);
 }
 
 /* join root and rest with a backslash */
@@ -7354,10 +7357,14 @@ _check_dirW(WCHAR *src, WCHAR *dest)
     WCHAR src_resolved[MAX_PATH] = L"";
 
     /* dest_parent = os.path.dirname(dest) */
-    wcscpy(dest_parent, dest);
-    _dirnameW(dest_parent);
+    if (wcscpy_s(dest_parent, MAX_PATH, dest) ||
+        _dirnameW(dest_parent)) {
+        return 0;
+    }
     /* src_resolved = os.path.join(dest_parent, src) */
-    _joinW(src_resolved, dest_parent, src);
+    if (_joinW(src_resolved, dest_parent, src)) {
+        return 0;
+    }
     return (
         GetFileAttributesExW(src_resolved, GetFileExInfoStandard, &src_info)
         && src_info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY
@@ -7432,15 +7439,10 @@ os_symlink_impl(PyObject *module, path_t *src, path_t *dst,
         }
 #endif
 
-    if ((src->narrow && dst->wide) || (src->wide && dst->narrow)) {
-        PyErr_SetString(PyExc_ValueError,
-            "symlink: src and dst must be the same type");
-        return NULL;
-    }
 
 #ifdef MS_WINDOWS
 
-    Py_BEGIN_ALLOW_THREADS
+    _Py_BEGIN_SUPPRESS_IPH
     if (dst->wide) {
         /* if src is a directory, ensure target_is_directory==1 */
         target_is_directory |= _check_dirW(src->wide, dst->wide);
@@ -7453,13 +7455,19 @@ os_symlink_impl(PyObject *module, path_t *src, path_t *dst,
         result = Py_CreateSymbolicLinkA(dst->narrow, src->narrow,
                                         target_is_directory);
     }
-    Py_END_ALLOW_THREADS
+    _Py_END_SUPPRESS_IPH
 
     if (!result)
         return path_error2(src, dst);
 
 #else
 
+    if ((src->narrow && dst->wide) || (src->wide && dst->narrow)) {
+        PyErr_SetString(PyExc_ValueError,
+            "symlink: src and dst must be the same type");
+        return NULL;
+    }
+
     Py_BEGIN_ALLOW_THREADS
 #if HAVE_SYMLINKAT
     if (dir_fd != DEFAULT_DIR_FD)