From 0672c185f40104514e44e9c6a5fed9a857df34ca Mon Sep 17 00:00:00 2001 From: Daniel Santos Date: Wed, 29 May 2024 12:44:08 -0500 Subject: [PATCH] Leaky handles query --- .../Security/CWE/CWE-403/LeakyHandles.cpp | 62 +++++++++ .../Security/CWE/CWE-403/LeakyHandles.qhelp | 30 +++++ .../Security/CWE/CWE-403/LeakyHandles.ql | 119 ++++++++++++++++++ 3 files changed, 211 insertions(+) create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-403/LeakyHandles.cpp create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-403/LeakyHandles.qhelp create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-403/LeakyHandles.ql diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-403/LeakyHandles.cpp b/cpp/ql/src/experimental/Security/CWE/CWE-403/LeakyHandles.cpp new file mode 100644 index 000000000000..0dc7e519cbc1 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-403/LeakyHandles.cpp @@ -0,0 +1,62 @@ +#include +#include +#include + +int main(int argc, char** argv) +{ + if (argc <= 1) { + printf("[-] Please give me a target PID\n"); + return -1; + } + + HANDLE hUserToken, hUserProcess; + HANDLE hProcess, hThread, hFile; + STARTUPINFOA si; + PROCESS_INFORMATION pi; + + ZeroMemory(&si, sizeof(si)); + si.cb = sizeof(si); + ZeroMemory(&pi, sizeof(pi)); + + hFile = CreateFile(L"C:\\Windows\\System32\\version.dll", GENERIC_ALL, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + + if (hFile == INVALID_HANDLE_VALUE) + { + printf("[-] Failed to open file: %d\n", GetLastError()); + return -1; + } + + std::cout << "[+] File handle: " << std::hex << hFile << "\n"; + + hUserProcess = OpenProcess(PROCESS_QUERY_INFORMATION, false, atoi(argv[1])); + if (!OpenProcessToken(hUserProcess, TOKEN_ALL_ACCESS, &hUserToken)) { + printf("[-] Failed to open user process: %d\n", GetLastError()); + CloseHandle(hUserProcess); + return -1; + } + + hProcess = OpenProcess(PROCESS_ALL_ACCESS, TRUE, GetCurrentProcessId()); + if (hProcess == NULL) + { + std::cerr << "[-] Failed to open process\n"; + return 1; + } + std::cout << "[+] Process handle: " << std::hex << hProcess << "\n"; + + hThread = OpenThread(THREAD_ALL_ACCESS, TRUE, GetCurrentThreadId()); + if (hThread == NULL) { + std::cerr << "[-] Failed to open thread\n"; + return 1; + } + + std::cout << "[+] Thread handle: " << std::hex << hThread << "\n"; + + char cmd[] = "C:\\Windows\\System32\\notepad.exe"; + if (!CreateProcessAsUserA(hUserToken, NULL, + cmd, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) { + std::cerr << "[-] Failed to create process as user: " << std::hex << GetLastError() << "\n"; + return 1; + } + SuspendThread(hThread); + return 0; +} \ No newline at end of file diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-403/LeakyHandles.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-403/LeakyHandles.qhelp new file mode 100644 index 000000000000..d71cdab34495 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-403/LeakyHandles.qhelp @@ -0,0 +1,30 @@ + + + +

+ In Windows environments, handles to processes, threads, and files are used to manage and interact with system resources. These handles can be opened with various permissions, such as process or thread access rights, which control what operations can be performed using the handle. +

+

+ A common issue arises when these handles are not properly closed before creating new processes. If the function + CreateProcessAsUser + or one of its variants is called with handle inheritance enabled, the opened handles may be inherited by the newly created process. This situation can lead to a privileged handle being leaked to a child process, which could potentially be exploited by an attacker to escalate privileges. +

+

+ Ensuring proper management of handles, including closing them with + CloseHandle + before creating new processes, is crucial to prevent these types of vulnerabilities. By doing so, the risk of handle inheritance and the subsequent security implications can be mitigated. +

+ + +

Ensure that all handles are properly closed using CloseHandle before creating new processes with handle inheritance enabled.

+ + +

The following example demonstrates several erroneous uses of handle management in a Windows application.

+ + + +
  • + Leaked Handle Exploitation +
    • +     +     \ No newline at end of file diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-403/LeakyHandles.ql b/cpp/ql/src/experimental/Security/CWE/CWE-403/LeakyHandles.ql new file mode 100644 index 000000000000..c62f28d820d0 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-403/LeakyHandles.ql @@ -0,0 +1,119 @@ +/** + * @name Memory leak on failed call to realloc + * @description An unprivileged child process may gain access to restricted resources by inheriting sensitive handles + * from a privileged parent process. + * @kind problem + * @id cpp/windows-handle-leak + * @problem.severity error + * @precision low + * @tags security + * experimental + * external/cwe/cwe-403 + */ + +import cpp + +// Base class to identify function calls with required permissions +abstract class HandleOpeningCall extends FunctionCall { + // Method to check if the permissions argument has at least one of the specified permissions + predicate hasRequiredPermissions() { 1 != 1 } +} + +// Class to identify OpenProcess calls +class OpenProcessCall extends HandleOpeningCall { + OpenProcessCall() { this.getTarget().hasName("OpenProcess") } + + override predicate hasRequiredPermissions() { + exists(Expr permArg | + this.getArgument(0) = permArg and + ( + permArg.getValueText().indexOf("PROCESS_ALL_ACCESS") >= 0 or + permArg.getValueText().indexOf("PROCESS_CREATE_PROCESS") >= 0 or + permArg.getValueText().indexOf("PROCESS_CREATE_THREAD") >= 0 or + permArg.getValueText().indexOf("PROCESS_DUP_HANDLE") >= 0 or + permArg.getValueText().indexOf("PROCESS_VM_WRITE") >= 0 + ) + ) + } +} + +// Class to identify OpenThread calls +class OpenThreadCall extends HandleOpeningCall { + OpenThreadCall() { this.getTarget().hasName("OpenThread") } + + override predicate hasRequiredPermissions() { + exists(Expr permArg | + this.getArgument(0) = permArg and + ( + permArg.getValueText().indexOf("THREAD_ALL_ACCESS") >= 0 or + permArg.getValueText().indexOf("THREAD_DIRECT_IMPERSONATION") >= 0 or + permArg.getValueText().indexOf("THREAD_SET_CONTEXT") >= 0 + ) + ) + } +} + +// Class to identify CreateFile calls +class CreateFileCall extends HandleOpeningCall { + CreateFileCall() { + this.getTarget().hasName("CreateFile") or + this.getTarget().hasName("CreateFileA") or + this.getTarget().hasName("CreateFileW") + } + + override predicate hasRequiredPermissions() { + exists(Expr permArg | + this.getArgument(0) = permArg and + ( + permArg.getValueText().indexOf("GENERIC_WRITE") >= 0 or + permArg.getValueText().indexOf("FILE_GENERIC_WRITE") >= 0 or + permArg.getValueText().indexOf("WRITE_OWNER") >= 0 or + permArg.getValueText().indexOf("WRITE_DAC") >= 0 + ) + ) + } +} + +// Class to identify CreateProcessAsUser calls +class CreateProcessAsUserCall extends FunctionCall { + CreateProcessAsUserCall() { + this.getTarget().hasName("CreateProcessAsUser") or + this.getTarget().hasName("CreateProcessAsUserA") or + this.getTarget().hasName("CreateProcessAsUserW") + } + + // Method to check if the sixth argument is TRUE + predicate isHandleInheritanceEnabled() { + exists(Expr arg6 | this.getArgument(5) = arg6 and arg6.getValueText().toUpperCase() = "TRUE") + } +} + +// Class to identify CloseHandle calls +class CloseHandleCall extends FunctionCall { + CloseHandleCall() { this.getTarget().hasName("CloseHandle") } +} + +// Function to find if CreateProcessAsUser is preceded by a handle opening call within the same function +// and ensure CloseHandle is not called on the handle before CreateProcessAsUser +predicate hasPrecedingHandleOpeningWithoutClose( + CreateProcessAsUserCall createProcessAsUserCall, HandleOpeningCall handleOpeningCall +) { + createProcessAsUserCall.isHandleInheritanceEnabled() and + createProcessAsUserCall.getEnclosingFunction() = handleOpeningCall.getEnclosingFunction() and + handleOpeningCall.getLocation().getStartLine() < + createProcessAsUserCall.getLocation().getStartLine() and + handleOpeningCall.hasRequiredPermissions() and + not exists(CloseHandleCall closeHandleCall | + closeHandleCall.getEnclosingFunction() = handleOpeningCall.getEnclosingFunction() and + closeHandleCall.getArgument(0) = handleOpeningCall.getAnArgument() and + closeHandleCall.getLocation().getStartLine() < + createProcessAsUserCall.getLocation().getStartLine() + ) +} + +from CreateProcessAsUserCall createProcessCall, HandleOpeningCall handleOpeningCall +where hasPrecedingHandleOpeningWithoutClose(createProcessCall, handleOpeningCall) +select createProcessCall, handleOpeningCall, + "The " + handleOpeningCall.getTarget().getName() + + " may leak a privileged handle to a child process through the " + + createProcessCall.getTarget().getName() + "."