From 9e6408add9aef672e38adaa0503a2919a3722d26 Mon Sep 17 00:00:00 2001 From: Sebastian Lackner Date: Thu, 28 Aug 2014 05:48:06 +0200 Subject: [PATCH] Fix invalid memory access and handle leak in Fix_Free / Junction_Point patches. --- patches/Makefile | 5 +- ...eak-and-invalid-memory-access-in-Rem.patch | 47 +++++++++++++++++++ patches/ntdll-Fix_Free/definition | 2 +- ...dd-support-for-deleting-junction-poi.patch | 35 +++++++------- 4 files changed, 68 insertions(+), 21 deletions(-) create mode 100644 patches/ntdll-Fix_Free/0011-kernel32-Fix-a-leak-and-invalid-memory-access-in-Rem.patch diff --git a/patches/Makefile b/patches/Makefile index 17ebed3b..fdadfe55 100644 --- a/patches/Makefile +++ b/patches/Makefile @@ -524,7 +524,7 @@ ntdll-Fix_Alignment.ok: # Patchset ntdll-Fix_Free # | # | Included patches: -# | * Fix unintentional leaks with ntdll internals [by Erich E. Hoover] +# | * Fix unintentional leaks with ntdll internals [rev 2, by Erich E. Hoover] # | # | Modified files: # | * dlls/kernel32/path.c, dlls/kernel32/volume.c, dlls/ntdll/directory.c, dlls/ntdll/file.c, dlls/ntdll/loader.c @@ -541,8 +541,9 @@ ntdll-Fix_Free.ok: $(call APPLY_FILE,ntdll-Fix_Free/0008-ntdll-Fix-leak-on-STATUS_NO_SUCH_FILE-for-certain-di.patch) $(call APPLY_FILE,ntdll-Fix_Free/0009-kernel32-Fix-leak-on-STATUS_NO_SUCH_FILE-in-RemoveDi.patch) $(call APPLY_FILE,ntdll-Fix_Free/0010-kernel32-Fix-leak-on-STATUS_NO_SUCH_FILE-in-QueryDos.patch) + $(call APPLY_FILE,ntdll-Fix_Free/0011-kernel32-Fix-a-leak-and-invalid-memory-access-in-Rem.patch) @( \ - echo '+ { "ntdll-Fix_Free", "Erich E. Hoover", "Fix unintentional leaks with ntdll internals" },'; \ + echo '+ { "ntdll-Fix_Free", "Erich E. Hoover", "Fix unintentional leaks with ntdll internals [rev 2]" },'; \ ) > ntdll-Fix_Free.ok # Patchset ntdll-Heap_FreeLists diff --git a/patches/ntdll-Fix_Free/0011-kernel32-Fix-a-leak-and-invalid-memory-access-in-Rem.patch b/patches/ntdll-Fix_Free/0011-kernel32-Fix-a-leak-and-invalid-memory-access-in-Rem.patch new file mode 100644 index 00000000..2eccf278 --- /dev/null +++ b/patches/ntdll-Fix_Free/0011-kernel32-Fix-a-leak-and-invalid-memory-access-in-Rem.patch @@ -0,0 +1,47 @@ +From 09d194aee9e84242a2843711947a72426fc8678c Mon Sep 17 00:00:00 2001 +From: Sebastian Lackner +Date: Thu, 28 Aug 2014 05:36:01 +0200 +Subject: kernel32: Fix a leak and invalid memory access in RemoveDirectoryW. + +NtClose( handle ) was missing on the error path, besides that unix_name is +not always initialized, and might contain garbage values - don't run +RtlFreeAnsiString in this case. +--- + dlls/kernel32/path.c | 15 +++++++++------ + 1 file changed, 9 insertions(+), 6 deletions(-) + +diff --git a/dlls/kernel32/path.c b/dlls/kernel32/path.c +index eeba48a..593cc1d 100644 +--- a/dlls/kernel32/path.c ++++ b/dlls/kernel32/path.c +@@ -1612,18 +1612,21 @@ BOOL WINAPI RemoveDirectoryW( LPCWSTR path ) + status = NtOpenFile( &handle, DELETE, &attr, &io, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + FILE_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT ); +- if (status == STATUS_SUCCESS) +- status = wine_nt_to_unix_file_name( &nt_name, &unix_name, FILE_OPEN, FALSE ); +- RtlFreeUnicodeString( &nt_name ); +- + if (status != STATUS_SUCCESS) + { + SetLastError( RtlNtStatusToDosError(status) ); +- RtlFreeAnsiString( &unix_name ); ++ RtlFreeUnicodeString( &nt_name ); + return FALSE; + } + +- if (!(ret = (rmdir( unix_name.Buffer ) != -1))) FILE_SetDosError(); ++ status = wine_nt_to_unix_file_name( &nt_name, &unix_name, FILE_OPEN, FALSE ); ++ RtlFreeUnicodeString( &nt_name ); ++ ++ if (status != STATUS_SUCCESS) ++ SetLastError( RtlNtStatusToDosError(status) ); ++ else if (!(ret = (rmdir( unix_name.Buffer ) != -1))) ++ FILE_SetDosError(); ++ + RtlFreeAnsiString( &unix_name ); + NtClose( handle ); + return ret; +-- +1.7.9.5 + diff --git a/patches/ntdll-Fix_Free/definition b/patches/ntdll-Fix_Free/definition index 99e72f2b..803a4c83 100644 --- a/patches/ntdll-Fix_Free/definition +++ b/patches/ntdll-Fix_Free/definition @@ -1,4 +1,4 @@ Author: Erich E. Hoover Subject: Fix unintentional leaks with ntdll internals -Revision: 1 +Revision: 2 Fixes: Fix unintentional leaks with ntdll internals diff --git a/patches/ntdll-Junction_Points/0005-kernel32-ntdll-Add-support-for-deleting-junction-poi.patch b/patches/ntdll-Junction_Points/0005-kernel32-ntdll-Add-support-for-deleting-junction-poi.patch index 246aa2eb..c51493d2 100644 --- a/patches/ntdll-Junction_Points/0005-kernel32-ntdll-Add-support-for-deleting-junction-poi.patch +++ b/patches/ntdll-Junction_Points/0005-kernel32-ntdll-Add-support-for-deleting-junction-poi.patch @@ -21,29 +21,28 @@ index eeba48a..b0b5ae9 100644 OBJECT_ATTRIBUTES attr; UNICODE_STRING nt_name; ANSI_STRING unix_name; -@@ -1614,16 +1615,23 @@ BOOL WINAPI RemoveDirectoryW( LPCWSTR path ) - FILE_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT ); - if (status == STATUS_SUCCESS) - status = wine_nt_to_unix_file_name( &nt_name, &unix_name, FILE_OPEN, FALSE ); +@@ -1620,13 +1621,21 @@ BOOL WINAPI RemoveDirectoryW( LPCWSTR path ) + } + + status = wine_nt_to_unix_file_name( &nt_name, &unix_name, FILE_OPEN, FALSE ); - RtlFreeUnicodeString( &nt_name ); if (status != STATUS_SUCCESS) - { -+ RtlFreeUnicodeString( &nt_name ); SetLastError( RtlNtStatusToDosError(status) ); - RtlFreeAnsiString( &unix_name ); - return FALSE; - } - -- if (!(ret = (rmdir( unix_name.Buffer ) != -1))) FILE_SetDosError(); -+ status = NtQueryAttributesFile( &attr, &info ); -+ RtlFreeUnicodeString( &nt_name ); -+ if ((info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) && (info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY)) -+ ret = (unlink( unix_name.Buffer ) != -1); +- else if (!(ret = (rmdir( unix_name.Buffer ) != -1))) +- FILE_SetDosError(); + else -+ ret = (rmdir( unix_name.Buffer ) != -1); -+ if (!ret) FILE_SetDosError(); -+ ++ { ++ status = NtQueryAttributesFile( &attr, &info ); ++ if (status == STATUS_SUCCESS && (info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) && ++ (info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY)) ++ ret = (unlink( unix_name.Buffer ) != -1); ++ else ++ ret = (rmdir( unix_name.Buffer ) != -1); ++ if (!ret) FILE_SetDosError(); ++ } + ++ RtlFreeUnicodeString( &nt_name ); RtlFreeAnsiString( &unix_name ); NtClose( handle ); return ret;