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.
keepassxreboot · Apr 29, 2024 · f812f0a · f812f0a
1 parent 195e5b5
commit f812f0a
Showing 1 changed file with 26 additions and 5 deletions.
diff --git 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<PSID>(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<PACL>(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