From 94df540ffe571d2fe0fe4ce1ff163af2541742bd Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Tue, 23 Apr 2024 20:53:12 -0400 Subject: [PATCH] Further prevent ability to access memory on Windows * Restrict access to changing DACL's after the process is started. This prevents the creator of the keepassxc.exe process from simply adding the permission to read memory back to the DACL list. * Verified using System Informer. --- src/core/Bootstrap.cpp | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/core/Bootstrap.cpp b/src/core/Bootstrap.cpp index 4b1bc90c9..739e3446f 100644 --- a/src/core/Bootstrap.cpp +++ b/src/core/Bootstrap.cpp @@ -129,6 +129,8 @@ namespace Bootstrap DWORD cbBufferSize = 0; PSID pLocalSystemSid = nullptr; DWORD pLocalSystemSidSize = SECURITY_MAX_SID_SIZE; + PSID pOwnerRightsSid = nullptr; + DWORD pOwnerRightsSidSize = SECURITY_MAX_SID_SIZE; // Access control list PACL pACL = nullptr; @@ -165,9 +167,20 @@ namespace Bootstrap goto Cleanup; } + // Retrieve CreaterOwnerRights SID + pOwnerRightsSid = static_cast(HeapAlloc(GetProcessHeap(), 0, pOwnerRightsSidSize)); + if (pOwnerRightsSid == nullptr) { + goto Cleanup; + } + + if (!CreateWellKnownSid(WinCreatorOwnerRightsSid, nullptr, pOwnerRightsSid, &pOwnerRightsSidSize)) { + auto error = GetLastError(); + goto Cleanup; + } + // Calculate the amount of memory that must be allocated for the DACL cbACL = sizeof(ACL) + sizeof(ACCESS_ALLOWED_ACE) + GetLengthSid(pTokenUser->User.Sid) - + sizeof(ACCESS_ALLOWED_ACE) + GetLengthSid(pLocalSystemSid); + + sizeof(ACCESS_ALLOWED_ACE) + GetLengthSid(pLocalSystemSid) + GetLengthSid(pOwnerRightsSid); // Create and initialize an ACL pACL = static_cast(HeapAlloc(GetProcessHeap(), 0, cbACL)); @@ -189,6 +202,11 @@ namespace Bootstrap goto Cleanup; } + // Explicitly set "Process Owner" rights to Read Only. The default is Full Control. + if (!AddAccessAllowedAce(pACL, ACL_REVISION, READ_CONTROL, pOwnerRightsSid)) { + goto Cleanup; + } + #ifdef WITH_XC_SSHAGENT // OpenSSH for Windows ssh-agent service is running as LocalSystem if (!AddAccessAllowedAce(pACL, @@ -213,16 +231,19 @@ namespace Bootstrap Cleanup: - if (pACL != nullptr) { + if (pACL) { HeapFree(GetProcessHeap(), 0, pACL); } - if (pLocalSystemSid != nullptr) { + if (pLocalSystemSid) { HeapFree(GetProcessHeap(), 0, pLocalSystemSid); } - if (pTokenUser != nullptr) { + if (pOwnerRightsSid) { + HeapFree(GetProcessHeap(), 0, pOwnerRightsSid); + } + if (pTokenUser) { HeapFree(GetProcessHeap(), 0, pTokenUser); } - if (hToken != nullptr) { + if (hToken) { CloseHandle(hToken); } #endif