bpo-33001: Prevent buffer overrun in os.symlink (GH-5989)

This commit is contained in:
Steve Dower 2018-03-05 14:26:08 -08:00 committed by GitHub
parent 4c19b95734
commit 6921e73e33
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 74 additions and 29 deletions

View File

@ -2164,6 +2164,40 @@ class Win32SymlinkTests(unittest.TestCase):
target = os.readlink(r'C:\Users\All Users')
self.assertTrue(os.path.samefile(target, r'C:\ProgramData'))
def test_buffer_overflow(self):
# Older versions would have a buffer overflow when detecting
# whether a link source was a directory. This test ensures we
# no longer crash, but does not otherwise validate the behavior
segment = 'X' * 27
path = os.path.join(*[segment] * 10)
test_cases = [
# overflow with absolute src
('\\' + path, segment),
# overflow dest with relative src
(segment, path),
# overflow when joining src
(path[:180], path[:180]),
]
for src, dest in test_cases:
try:
os.symlink(src, dest)
except FileNotFoundError:
pass
else:
try:
os.remove(dest)
except OSError:
pass
# Also test with bytes, since that is a separate code path.
try:
os.symlink(os.fsencode(src), os.fsencode(dest))
except FileNotFoundError:
pass
else:
try:
os.remove(dest)
except OSError:
pass
@unittest.skipUnless(sys.platform == "win32", "Win32 specific tests")
class Win32JunctionTests(unittest.TestCase):

View File

@ -0,0 +1 @@
Minimal fix to prevent buffer overrun in os.symlink on Windows

View File

@ -7471,7 +7471,7 @@ win_readlink(PyObject *self, PyObject *args, PyObject *kwargs)
#if defined(MS_WINDOWS)
/* Grab CreateSymbolicLinkW dynamically from kernel32 */
static DWORD (CALLBACK *Py_CreateSymbolicLinkW)(LPCWSTR, LPCWSTR, DWORD) = NULL;
static BOOLEAN (CALLBACK *Py_CreateSymbolicLinkW)(LPCWSTR, LPCWSTR, DWORD) = NULL;
static int
check_CreateSymbolicLink(void)
@ -7486,47 +7486,51 @@ check_CreateSymbolicLink(void)
return Py_CreateSymbolicLinkW != NULL;
}
/* 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;
}
/* Is this path absolute? */
static int
_is_absW(const WCHAR *path)
{
return path[0] == L'\\' || path[0] == L'/' || path[1] == L':';
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);
}
/* Return True if the path at src relative to dest is a directory */
@ -7538,10 +7542,14 @@ _check_dirW(LPCWSTR src, LPCWSTR 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
@ -7597,19 +7605,15 @@ 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 src is a directory, ensure target_is_directory==1 */
target_is_directory |= _check_dirW(src->wide, dst->wide);
result = Py_CreateSymbolicLinkW(dst->wide, src->wide,
target_is_directory);
_Py_END_SUPPRESS_IPH
Py_END_ALLOW_THREADS
if (!result)
@ -7617,6 +7621,12 @@ os_symlink_impl(PyObject *module, path_t *src, path_t *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)