From d0ebaff7b4aeedbe1898fd38a9921ddc64dda99b Mon Sep 17 00:00:00 2001 From: rockihack Date: Sat, 25 Feb 2017 01:12:01 +0100 Subject: [PATCH 1/4] Prevent memory dumps on windows. --- src/core/Tools.cpp | 105 +++++++++++++++++++++++++++++++++++++++++++++ src/core/Tools.h | 1 + 2 files changed, 106 insertions(+) diff --git a/src/core/Tools.cpp b/src/core/Tools.cpp index bc63bf13..57b77878 100644 --- a/src/core/Tools.cpp +++ b/src/core/Tools.cpp @@ -1,5 +1,6 @@ /* * Copyright (C) 2012 Felix Geyer + * Copyright (C) 2017 Lennart Glauer * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -27,6 +28,7 @@ #ifdef Q_OS_WIN #include // for Sleep(), SetDllDirectoryA() and SetSearchPathMode() +#include #endif #ifdef Q_OS_UNIX @@ -226,6 +228,10 @@ void disableCoreDumps() success = success && (ptrace(PT_DENY_ATTACH, 0, 0, 0) == 0); #endif +#ifdef Q_OS_WIN + success = success && createWindowsDACL(); +#endif + if (!success) { qWarning("Unable to disable core dumps."); } @@ -240,4 +246,103 @@ void setupSearchPaths() #endif } +// +// Prevent memory dumps without admin privileges. +// MiniDumpWriteDump function requires +// PROCESS_QUERY_INFORMATION and PROCESS_VM_READ +// see: https://msdn.microsoft.com/en-us/library/windows/desktop/ms680360%28v=vs.85%29.aspx +// +bool createWindowsDACL() +{ + bool bSuccess = false; + + // Access control list + PACL pACL = nullptr; + DWORD cbACL = 0; + + // Security identifiers + PSID pSIDAdmin = nullptr; + PSID pSIDSystem = nullptr; + SID_IDENTIFIER_AUTHORITY SIDAuthNT = SECURITY_NT_AUTHORITY; + + // Create a SID for the BUILTIN\Administrators group + if (!AllocateAndInitializeSid( + &SIDAuthNT, + 2, + SECURITY_BUILTIN_DOMAIN_RID, + DOMAIN_ALIAS_RID_ADMINS, + 0, 0, 0, 0, 0, 0, + &pSIDAdmin + )) { + goto Cleanup; + } + + // Create a SID for the System group + if (!AllocateAndInitializeSid( + &SIDAuthNT, + 1, + SECURITY_LOCAL_SYSTEM_RID, + 0, 0, 0, 0, 0, 0, 0, + &pSIDSystem + )) { + goto Cleanup; + } + + cbACL = sizeof(ACL) + + sizeof(ACCESS_ALLOWED_ACE) + GetLengthSid(pSIDAdmin) + + sizeof(ACCESS_ALLOWED_ACE) + GetLengthSid(pSIDSystem); + + pACL = static_cast(HeapAlloc(GetProcessHeap(), 0, cbACL)); + if (pACL == nullptr) { + goto Cleanup; + } + + // Initialize access control list + if (!InitializeAcl(pACL, cbACL, ACL_REVISION)) { + goto Cleanup; + } + + // Add allowed access control entries, everything else is denied + if (!AddAccessAllowedAce( + pACL, + ACL_REVISION, + SYNCHRONIZE | PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_TERMINATE, // protected process + pSIDAdmin + )) { + goto Cleanup; + } + if (!AddAccessAllowedAce( + pACL, + ACL_REVISION, + PROCESS_ALL_ACCESS, + pSIDSystem + )) { + goto Cleanup; + } + + // Update discretionary access control list + bSuccess = ERROR_SUCCESS == SetSecurityInfo( + GetCurrentProcess(), // object handle + SE_KERNEL_OBJECT, // type of object + DACL_SECURITY_INFORMATION, // change only the objects DACL + nullptr, nullptr, // do not change owner or group + pACL, // DACL specified + nullptr // do not change SACL + ); + +Cleanup: + + if (pSIDAdmin != nullptr) { + FreeSid(pSIDAdmin); + } + if (pSIDSystem != nullptr) { + FreeSid(pSIDSystem); + } + if (pACL != nullptr) { + HeapFree(GetProcessHeap(), 0, pACL); + } + + return bSuccess; +} + } // namespace Tools diff --git a/src/core/Tools.h b/src/core/Tools.h index 65df1ea4..ba55054a 100644 --- a/src/core/Tools.h +++ b/src/core/Tools.h @@ -41,6 +41,7 @@ void sleep(int ms); void wait(int ms); void disableCoreDumps(); void setupSearchPaths(); +bool createWindowsDACL(); template RandomAccessIterator binaryFind(RandomAccessIterator begin, RandomAccessIterator end, const T& value) From 153dc620c8d32e35c6ea0a46a8a84c59a0d0a69e Mon Sep 17 00:00:00 2001 From: rockihack Date: Sat, 25 Feb 2017 01:35:47 +0100 Subject: [PATCH 2/4] Add #ifdef Q_OS_WIN guard. --- src/core/Tools.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/Tools.cpp b/src/core/Tools.cpp index 57b77878..a6bff32f 100644 --- a/src/core/Tools.cpp +++ b/src/core/Tools.cpp @@ -256,6 +256,7 @@ bool createWindowsDACL() { bool bSuccess = false; +#ifdef Q_OS_WIN // Access control list PACL pACL = nullptr; DWORD cbACL = 0; @@ -341,6 +342,7 @@ Cleanup: if (pACL != nullptr) { HeapFree(GetProcessHeap(), 0, pACL); } +#endif return bSuccess; } From 6d69f0b547a77676c80e9b51ac2bcd3f05665b36 Mon Sep 17 00:00:00 2001 From: rockihack Date: Sun, 26 Feb 2017 22:59:21 +0100 Subject: [PATCH 3/4] Grant minimal access rights to the user associated with the process token. --- src/core/Tools.cpp | 89 ++++++++++++++++++++++++---------------------- 1 file changed, 47 insertions(+), 42 deletions(-) diff --git a/src/core/Tools.cpp b/src/core/Tools.cpp index a6bff32f..54cca29c 100644 --- a/src/core/Tools.cpp +++ b/src/core/Tools.cpp @@ -248,8 +248,7 @@ void setupSearchPaths() // // Prevent memory dumps without admin privileges. -// MiniDumpWriteDump function requires -// PROCESS_QUERY_INFORMATION and PROCESS_VM_READ +// MiniDumpWriteDump function requires PROCESS_QUERY_INFORMATION and PROCESS_VM_READ // see: https://msdn.microsoft.com/en-us/library/windows/desktop/ms680360%28v=vs.85%29.aspx // bool createWindowsDACL() @@ -257,48 +256,62 @@ bool createWindowsDACL() bool bSuccess = false; #ifdef Q_OS_WIN + // Process token and user + HANDLE hToken = nullptr; + PTOKEN_USER pTokenUser = nullptr; + DWORD cbBufferSize = 0; + // Access control list PACL pACL = nullptr; DWORD cbACL = 0; - // Security identifiers - PSID pSIDAdmin = nullptr; - PSID pSIDSystem = nullptr; - SID_IDENTIFIER_AUTHORITY SIDAuthNT = SECURITY_NT_AUTHORITY; - - // Create a SID for the BUILTIN\Administrators group - if (!AllocateAndInitializeSid( - &SIDAuthNT, - 2, - SECURITY_BUILTIN_DOMAIN_RID, - DOMAIN_ALIAS_RID_ADMINS, - 0, 0, 0, 0, 0, 0, - &pSIDAdmin + // Open the access token associated with the calling process + if (!OpenProcessToken( + GetCurrentProcess(), + TOKEN_QUERY, + &hToken )) { goto Cleanup; } - // Create a SID for the System group - if (!AllocateAndInitializeSid( - &SIDAuthNT, - 1, - SECURITY_LOCAL_SYSTEM_RID, - 0, 0, 0, 0, 0, 0, 0, - &pSIDSystem + // Retrieve the token information in a TOKEN_USER structure + GetTokenInformation( + hToken, + TokenUser, // request for a TOKEN_USER structure + nullptr, + 0, + &cbBufferSize + ); + + pTokenUser = static_cast(HeapAlloc(GetProcessHeap(), 0, cbBufferSize)); + if (pTokenUser == nullptr) { + goto Cleanup; + } + + if (!GetTokenInformation( + hToken, + TokenUser, + pTokenUser, + cbBufferSize, + &cbBufferSize )) { goto Cleanup; } + if (!IsValidSid(pTokenUser->User.Sid)) { + goto Cleanup; + } + + // Calculate the amount of memory that must be allocated for the DACL cbACL = sizeof(ACL) - + sizeof(ACCESS_ALLOWED_ACE) + GetLengthSid(pSIDAdmin) - + sizeof(ACCESS_ALLOWED_ACE) + GetLengthSid(pSIDSystem); + + sizeof(ACCESS_ALLOWED_ACE) + GetLengthSid(pTokenUser->User.Sid); + // Create and initialize an ACL pACL = static_cast(HeapAlloc(GetProcessHeap(), 0, cbACL)); if (pACL == nullptr) { goto Cleanup; } - // Initialize access control list if (!InitializeAcl(pACL, cbACL, ACL_REVISION)) { goto Cleanup; } @@ -307,21 +320,13 @@ bool createWindowsDACL() if (!AddAccessAllowedAce( pACL, ACL_REVISION, - SYNCHRONIZE | PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_TERMINATE, // protected process - pSIDAdmin - )) { - goto Cleanup; - } - if (!AddAccessAllowedAce( - pACL, - ACL_REVISION, - PROCESS_ALL_ACCESS, - pSIDSystem + SYNCHRONIZE | PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_TERMINATE, // same as protected process + pTokenUser->User.Sid // pointer to the trustee's SID )) { goto Cleanup; } - // Update discretionary access control list + // Set discretionary access control list bSuccess = ERROR_SUCCESS == SetSecurityInfo( GetCurrentProcess(), // object handle SE_KERNEL_OBJECT, // type of object @@ -333,15 +338,15 @@ bool createWindowsDACL() Cleanup: - if (pSIDAdmin != nullptr) { - FreeSid(pSIDAdmin); - } - if (pSIDSystem != nullptr) { - FreeSid(pSIDSystem); - } if (pACL != nullptr) { HeapFree(GetProcessHeap(), 0, pACL); } + if (pTokenUser != nullptr) { + HeapFree(GetProcessHeap(), 0, pTokenUser); + } + if (hToken != nullptr) { + CloseHandle(hToken); + } #endif return bSuccess; From cdf54b07c5bbedb26cb8097eaecda8f7ce3fc0c7 Mon Sep 17 00:00:00 2001 From: rockihack Date: Thu, 2 Mar 2017 19:24:31 +0100 Subject: [PATCH 4/4] Add more detailed comment. --- src/core/Tools.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/core/Tools.cpp b/src/core/Tools.cpp index 54cca29c..a1bfcb0c 100644 --- a/src/core/Tools.cpp +++ b/src/core/Tools.cpp @@ -27,8 +27,8 @@ #include #ifdef Q_OS_WIN -#include // for Sleep(), SetDllDirectoryA() and SetSearchPathMode() -#include +#include // for Sleep(), SetDllDirectoryA(), SetSearchPathMode(), ... +#include // for SetSecurityInfo() #endif #ifdef Q_OS_UNIX @@ -247,9 +247,13 @@ void setupSearchPaths() } // -// Prevent memory dumps without admin privileges. -// MiniDumpWriteDump function requires PROCESS_QUERY_INFORMATION and PROCESS_VM_READ -// see: https://msdn.microsoft.com/en-us/library/windows/desktop/ms680360%28v=vs.85%29.aspx +// This function grants the user associated with the process token minimal access rights and +// denies everything else on Windows. This includes PROCESS_QUERY_INFORMATION and +// PROCESS_VM_READ access rights that are required for MiniDumpWriteDump() or ReadProcessMemory(). +// We do this using a discretionary access control list (DACL). Effectively this prevents +// crash dumps and disallows other processes from accessing our memory. This works as long +// as you do not have admin privileges, since then you are able to grant yourself the +// SeDebugPrivilege or SeTakeOwnershipPrivilege and circumvent the DACL. // bool createWindowsDACL() { @@ -277,7 +281,7 @@ bool createWindowsDACL() // Retrieve the token information in a TOKEN_USER structure GetTokenInformation( hToken, - TokenUser, // request for a TOKEN_USER structure + TokenUser, nullptr, 0, &cbBufferSize