Skip to content

Add mod: Unlock Office Files for Upload#4284

Open
loerei wants to merge 2 commits into
ramensoftware:mainfrom
loerei:add-unlock-office-file-upload
Open

Add mod: Unlock Office Files for Upload#4284
loerei wants to merge 2 commits into
ramensoftware:mainfrom
loerei:add-unlock-office-file-upload

Conversation

@loerei
Copy link
Copy Markdown

@loerei loerei commented Jun 4, 2026

Adds a new mod that transparently unlocks Office files that are locked by Word/Excel/PowerPoint when another process tries to read them for upload or attachment.

What it does

When a .docx, .xlsx, .pptx (or legacy variants) is open in Word/Excel/PowerPoint, attempting to upload or attach it in Chrome, Edge, Outlook, Teams, Discord, etc. fails with:

This file is in use. Enter a new name or close the file that's open in another program.

This mod hooks CreateFileW in every process. When a read-only open on a tracked extension fails with ERROR_SHARING_VIOLATION, it:

  1. Makes a temp copy of the file in %TEMP%
  2. Redirects the open to the copy (caller gets a valid handle, no error)
  3. Hooks CloseHandle to delete the temp copy automatically when done

No user-visible prompts. No need to close Word first.

Details

  • Hooks: CreateFileW, CloseHandle (via kernel32.dll)
  • Target: @include * (all processes — needed since the uploader can be any app)
  • Extensions: Configurable in Windhawk settings; defaults cover .docx/.docm/.doc, .xlsx/.xlsm/.xls, .pptx/.pptm/.ppt
  • Limitation: The copy reflects the last saved state; unsaved edits in Word are not included
  • Source repo: https://github.com/loerei/windhawk-unlock-office-file-upload

@m417z
Copy link
Copy Markdown
Member

m417z commented Jun 4, 2026

Submission review

Note: This review was done by Claude, and then refined manually. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

For item 2, perhaps at least exclude the indexer service and other system processes such as explorer.exe. Also consider excluding processes running in session 0 and non-GUI processes.


Clever idea, and the core flow (let the open fail, copy on ERROR_SHARING_VIOLATION, redirect) is sound. The main concerns are the system-wide blast radius and the temp-file lifecycle — both can be solved at once by dropping the CloseHandle hook.

1. The global CloseHandle hook is a system-wide cost — and it's avoidable. With @include *, you've put a std::mutex lock + unordered_map lookup on every CloseHandle call in every process on the machine. CloseHandle is one of the hottest handle APIs (files, events, threads, registry keys, sockets…), so this is a real, always-on overhead for a feature that fires only on a rare upload of a locked Office file. You don't need the hook at all: open the temp copy with FILE_FLAG_DELETE_ON_CLOSE and the OS deletes it when the caller closes the handle. That removes the CloseHandle hook, the g_tempHandles map, the mutex, and TrackHandle/UntrackHandle entirely:

HANDLE hTemp = CreateFileW_Original(
    tempPath.c_str(), dwDesiredAccess,
    FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
    lpSecurityAttributes, dwCreationDisposition,
    dwFlagsAndAttributes | FILE_FLAG_DELETE_ON_CLOSE,
    hTemplateFile);

2. Temp files leak on process crash / abnormal exit. Wh_ModUninit runs only on a normal mod disable, not when the host process terminates (the uploader is closed, crashes, Explorer restarts, sign-out, etc.). Any process that exits with a redirected handle still open leaves a ~ul_* file in %TEMP% permanently. Same story for handles that get closed through a path that bypasses the kernel32 CloseHandle export (NtClose, some CRT _close paths) — your hook never sees them, so the file is never deleted. FILE_FLAG_DELETE_ON_CLOSE (item 1) fixes both cases: the OS deletes the temp on the final handle close, including the forced close the kernel does at process teardown. With that in place, the cleanup loop in Wh_ModUninit becomes unnecessary too.

3. @include * blast radius and unintended redirects. Beyond the cost of injecting into every process, the redirect logic fires for any read-only open that hits a sharing violation on a tracked extension — not just genuine uploads. The Windows Search indexer, antivirus, backup/thumbnail/preview handlers, etc. would silently be handed a throwaway temp copy instead of the real file (and then the copy disappears). That's a surprising side effect to push system-wide. Consider narrowing @include to the executables that actually upload/attach files (browsers, mail and chat clients) rather than *. If a curated list is genuinely infeasible for your use case, at least call out the tradeoff in the README. (After item 1, the only remaining global hook is CreateFileW, which is cheap on the success path since you return immediately when the original call succeeds — so narrowing scope is the remaining win here.)

Optional improvements

Minor polish — none of this affects users, so it's your call.

  • Data race on g_extensions during a settings change. IsTrackedExtension iterates g_extensions from the CreateFileW hook with no lock, while Wh_ModSettingsChangedLoadSettings does g_extensions.clear() + push_back on another thread. A reallocation mid-iteration can invalidate the iterator and crash — and with @include * this runs in every process at once on every settings change. Either guard g_extensions with a mutex, or build a fresh std::vector locally in LoadSettings and swap it in.
  • Type-safe hooking. WindhawkUtils::SetFunctionHook(pfnCreateFileW, CreateFileW_Hook, &CreateFileW_Original) avoids the void* casts and checks the prototype at compile time. (The raw Wh_SetFunctionHook form you used is accepted and appears in other mods, e.g. shadowplay-single-folder.wh.cpp, so this is just a preference.)
  • Weak temp-name uniqueness. GetTickCount() ^ (GetCurrentThreadId() << 16) collides easily — GetTickCount only changes every ~15 ms, so two opens on the same thread in quick succession get the same name. GetTempFileNameW gives you a guaranteed-unique name in one call.
  • ::tolower on wchar_t (lines 82, 141) is the narrow-char function; passing a wchar_t > 127 to it is technically out of range and it won't fold non-ASCII correctly. Use towlower (from <cwctype>). For your ASCII extensions it happens to work, but towlower is the correct call.
  • Redundant NULL check. Wh_GetStringSetting never returns NULL — it returns L"" when unset — so !val in LoadSettings is dead; the val[0] == L'\0' check alone is enough.
  • Access-rights guard only checks GENERIC_WRITE. A write-intent open that uses FILE_WRITE_DATA/FILE_GENERIC_WRITE without the generic bit would still be redirected to a temp copy that's then deleted — silent write loss. Edge case (uploads are read-only), but cheap to also exclude those bits if you want to be safe.
  • swprintf_s relies on a transitive include from <windows.h>; add <cstdio> to be explicit.

Functionality notes

Non-critical observations about the feature behavior itself.

  • Silent stale-data substitution. The uploader receives the last saved version with no indication that it differs from what's on screen. You document this in the README and there's no way to read Word's unsaved buffer, so it's inherent — but a user could unknowingly attach an outdated file. Worth considering whether a one-time log/notice is warranted; not a blocker.
  • Whole-file copy is synchronous and inside CreateFileW. Every locked read of a tracked file copies the entire document into %TEMP% while the caller blocks in CreateFileW. For a large spreadsheet this is a noticeable stall plus a transient disk-space spike (and a momentary plaintext copy of the document in %TEMP%). Acceptable given the goal — just an FYI.
  • Hook coverage. Only CreateFileW is intercepted. Uploaders that open via CreateFile2, NtCreateFile, or structured-storage paths that bypass the kernel32 export won't be helped, and the kernel32 CreateFileW export may miss internal kernelbase callers within a process. For typical app-level uploaders this is fine; just know the boundary.
  • Read-deny holders. The copy only succeeds when the file is actually readable by a permissive opener (Word's usual FILE_SHARE_READ lock). If the holder denies reads outright, CopyFileW also fails and you correctly fall back to returning the original sharing violation — the mod simply can't help in that case, which is the right behavior.

…, towlower, GetTempFileNameW, session 0 skip, widen write guard
@loerei
Copy link
Copy Markdown
Author

loerei commented Jun 4, 2026

All required and optional points addressed in v0.2. Summary of changes:

1. Dropped the CloseHandle hook entirely — replaced with FILE_FLAG_DELETE_ON_CLOSE

The CloseHandle hook, the g_tempHandles map, the mutex, and TrackHandle/UntrackHandle are all gone. Instead the temp file is opened with FILE_FLAG_DELETE_ON_CLOSE — the OS handles deletion when the last handle closes, including on process crash or abnormal exit. Wh_ModUninit no longer needs a cleanup loop.

2. Narrowed scope

  • Added @exclude SearchIndexer.exe and @exclude explorer.exe to the metadata.
  • Added a session 0 check at the top of Wh_ModInit — if ProcessIdToSessionId returns session 0, the mod returns FALSE immediately and installs no hooks in that process.

3. Fixed g_extensions data race

LoadSettings now builds a fresh std::vector locally, then swaps it in under a std::unique_lock. IsTrackedExtension takes a std::shared_lock. Both share a std::shared_mutex g_extensionsMutex.

4. Fixed ::tolowertowlower

Both GetExtension and LoadSettings now use a lambda with towlower (from <cwctype>) via static_cast<wchar_t>(towlower(c)).

5. Fixed temp-name uniqueness — switched to GetTempFileNameW

MakeTempPath now calls GetTempFileNameW(tempDir, L"ul_", 0, out) which gives a kernel-guaranteed unique name and creates the placeholder file. CopyFileW with bFailIfExists=FALSE overwrites it.

6. Removed redundant !val check

Wh_GetStringSetting never returns NULL; the !val guard is dropped, leaving only val[0] == L'\0'.

7. Widened write-bits guard

The access check now covers GENERIC_WRITE | FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA | FILE_APPEND_DATA to avoid silently redirecting write-intent opens that use specific bits instead of the generic alias.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants