From 8a1b5f46145cc8be56ce09eec1513db079b86bc7 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 14 May 2016 21:18:00 -0500 Subject: [PATCH 1/1] win32_apply.c: better workaround for access denied bug when creating ADS Removing directory DACLs was not really a good idea, since this state can too easily remain after extraction. Instead, since the unexpected STATUS_ACCESS_DENIED error only occurs when creating an empty named data stream, we can avoid the error by requesting no permissions on the handle. It happens that no permissions are needed anyway. --- NEWS | 3 +++ src/win32_apply.c | 38 ++++++++++++++------------------------ 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/NEWS b/NEWS index 30dea771..4bda7ea3 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,9 @@ Version 1.9.2-BETA: On UNIX, wimlib can now overwrite readonly files when extracting. + On Windows, fixed a bug where wimlib could leave a null DACL (a.k.a. "no + NTFS permissions") set on some existing directories after extraction. + On Windows, when applying a WIM image in "WIMBoot mode" when the WOF driver is not loaded, wimlib can now correctly register a new WIM file with the target volume when the target volume previously had had WIM diff --git a/src/win32_apply.c b/src/win32_apply.c index 6f9e41b9..d9bf63c2 100644 --- a/src/win32_apply.c +++ b/src/win32_apply.c @@ -1471,10 +1471,11 @@ retry: /* * Create a nondirectory file or named data stream at the current path, * superseding any that already exists at that path. If successful, return an - * open handle to the file or named data stream. + * open handle to the file or named data stream with the requested permissions. */ static int -supersede_file_or_stream(struct win32_apply_ctx *ctx, HANDLE *h_ret) +supersede_file_or_stream(struct win32_apply_ctx *ctx, DWORD perms, + HANDLE *h_ret) { NTSTATUS status; bool retried = false; @@ -1483,7 +1484,7 @@ supersede_file_or_stream(struct win32_apply_ctx *ctx, HANDLE *h_ret) * FILE_ATTRIBUTE_ENCRYPTED doesn't get set before we want it to be. */ retry: status = do_create_file(h_ret, - GENERIC_READ | GENERIC_WRITE | DELETE, + perms, NULL, FILE_ATTRIBUTE_SYSTEM, FILE_CREATE, @@ -1595,7 +1596,13 @@ create_empty_streams(const struct wim_dentry *dentry, build_extraction_path_with_ads(dentry, ctx, strm->stream_name, utf16le_len_chars(strm->stream_name)); - ret = supersede_file_or_stream(ctx, &h); + /* + * Note: do not request any permissions on the handle. + * Otherwise, we may encounter a Windows bug where the + * parent directory DACL denies read access to the new + * named data stream, even when using backup semantics! + */ + ret = supersede_file_or_stream(ctx, 0, &h); build_extraction_path(dentry, ctx); @@ -1683,25 +1690,6 @@ retry: sizeof(basic_info), FileBasicInformation); } - - /* Also try to remove the directory's DACL. This isn't supposed - * to be necessary because we *always* use backup semantics. - * However, there is a case where NtCreateFile() fails with - * STATUS_ACCESS_DENIED when creating a named data stream that - * was just deleted, using a directory-relative open. I have no - * idea why Windows is broken in this case. */ - if (!(ctx->common.extract_flags & WIMLIB_EXTRACT_FLAG_NO_ACLS)) { - static const SECURITY_DESCRIPTOR_RELATIVE desc = { - .Revision = SECURITY_DESCRIPTOR_REVISION1, - .Control = SE_SELF_RELATIVE | SE_DACL_PRESENT, - .Owner = 0, - .Group = 0, - .Sacl = 0, - .Dacl = 0, - }; - NtSetSecurityObject(h, DACL_SECURITY_INFORMATION, - (void *)&desc); - } } if (!dentry_is_root(dentry)) { @@ -1771,7 +1759,9 @@ create_nondirectory_inode(HANDLE *h_ret, const struct wim_dentry *dentry, build_extraction_path(dentry, ctx); - ret = supersede_file_or_stream(ctx, &h); + ret = supersede_file_or_stream(ctx, + GENERIC_READ | GENERIC_WRITE | DELETE, + &h); if (ret) goto out; -- 2.43.0