From 42b7a924f19762367e9fc055c85384246fde53e0 Mon Sep 17 00:00:00 2001 From: crazy hugsy Date: Mon, 25 Mar 2024 18:54:49 -0700 Subject: [PATCH] [`CFB::Driver`] Fix null input data bug (#30) * restored old behavior for data capture * fix str <-> wstr issue in utils * added screenshot to readme --- Broker/Source/Connectors/JsonQueue.cpp | 3 +- Broker/Source/DriverManager.cpp | 4 +- Common/Source/Utils.cpp | 52 +++---- Driver/Headers/Context.hpp | 192 ++++++++++++------------- Driver/Source/CapturedIrp.cpp | 97 ++++++------- Driver/Source/Entry.cpp | 8 +- README.md | 14 +- 7 files changed, 189 insertions(+), 181 deletions(-) diff --git a/Broker/Source/Connectors/JsonQueue.cpp b/Broker/Source/Connectors/JsonQueue.cpp index 91387b2..22ef36d 100644 --- a/Broker/Source/Connectors/JsonQueue.cpp +++ b/Broker/Source/Connectors/JsonQueue.cpp @@ -24,6 +24,7 @@ JsonQueue::Name() const Result JsonQueue::IrpCallback(CFB::Comms::CapturedIrp const& Irp) { + std::scoped_lock(m_Lock); m_Queue.push(std::make_unique(Irp)); return Ok(0); } @@ -31,12 +32,12 @@ JsonQueue::IrpCallback(CFB::Comms::CapturedIrp const& Irp) std::unique_ptr JsonQueue::Pop() { + std::scoped_lock(m_Lock); if ( m_Queue.empty() ) { return nullptr; } - std::scoped_lock(m_Lock); auto Irp = std::move(m_Queue.front()); m_Queue.pop(); diff --git a/Broker/Source/DriverManager.cpp b/Broker/Source/DriverManager.cpp index c47b12f..7ca65c6 100644 --- a/Broker/Source/DriverManager.cpp +++ b/Broker/Source/DriverManager.cpp @@ -304,7 +304,7 @@ DriverManager::ExecuteCommand(json const& Request) break; } - if ( RequestId != CFB::Comms::RequestId::GetPendingIrp ) + // if ( RequestId != CFB::Comms::RequestId::GetPendingIrp ) { info( "Request[%llu] %s => %s", @@ -312,7 +312,7 @@ DriverManager::ExecuteCommand(json const& Request) CFB::Utils::ToString(RequestId).c_str(), boolstr(Response["success"])); - dbg("Request[%llu] => %s", m_RequestNumber, Response.dump().c_str()); + info("Request[%llu] => %s", m_RequestNumber, Response.dump().c_str()); } return Ok(Response); diff --git a/Common/Source/Utils.cpp b/Common/Source/Utils.cpp index 723cb70..8e79cbe 100644 --- a/Common/Source/Utils.cpp +++ b/Common/Source/Utils.cpp @@ -173,40 +173,40 @@ IrqlToString(u32 type) #ifdef CFB_KERNEL_DRIVER #else std::string -ToString(std::wstring const& WideString) +ToString(std::wstring const& wide_string) { - // auto converter = std::wstring_convert >(); - // return converter.to_bytes(input); + int size_needed = + ::WideCharToMultiByte(CP_UTF8, 0, wide_string.data(), (int)wide_string.size(), nullptr, 0, nullptr, nullptr); - // HACK improve - std::string s; - std::for_each( - WideString.cbegin(), - WideString.cend(), - [&s](auto c) - { - s += (char)c; - }); - return s; + std::string str(size_needed, 0); + + if ( 0 == ::WideCharToMultiByte( + CP_UTF8, + 0, + wide_string.data(), + (int)wide_string.size(), + str.data(), + (int)str.size(), + nullptr, + nullptr) ) + { + str.clear(); + } + return str; } std::wstring -ToWideString(std::string const& String) +ToWideString(std::string const& str) { - // auto converter = std::wstring_convert >(); - // return converter.from_bytes(input); + int size_needed = ::MultiByteToWideChar(CP_UTF8, 0, str.data(), (int)str.size(), nullptr, 0); - // HACK improve + std::wstring wstr(size_needed, 0); - std::wstring s; - std::for_each( - String.cbegin(), - String.cend(), - [&s](auto c) - { - s += (wchar_t)c; - }); - return s; + if ( 0 == ::MultiByteToWideChar(CP_UTF8, 0, str.data(), (int)str.size(), wstr.data(), (int)wstr.size()) ) + { + wstr.clear(); + } + return wstr; } std::string diff --git a/Driver/Headers/Context.hpp b/Driver/Headers/Context.hpp index 8dc5909..c276e41 100644 --- a/Driver/Headers/Context.hpp +++ b/Driver/Headers/Context.hpp @@ -1,96 +1,96 @@ -#pragma once - -// clang-format off -#include "Common.hpp" -#include "DriverUtils.hpp" -#include "Log.hpp" - -#include "CapturedIrpManager.hpp" -#include "HookedDriverManager.hpp" -// clang-format on - - -#define CFB_MAX_HEXDUMP_BYTE 128 - -namespace Driver = CFB::Driver; -namespace Utils = CFB::Driver::Utils; - -struct GlobalContext -{ - /// - /// @brief Any critical read/write operation to the global context structure must acquire this lock. - /// - Utils::KQueuedSpinLock ContextLock; - - /// - /// @brief A pointer to the device object - /// - PDRIVER_OBJECT DriverObject; - - /// - /// @brief A pointer to the device object - /// - PDEVICE_OBJECT DeviceObject; - - /// - /// @brief A pointer to the EPROCESS of the broker. Not more than one handle to the - /// device is allowed. - /// - PEPROCESS Owner; - - /// - /// @brief Incremental session ID number. - /// - ULONG SessionId; - - /// - /// @brief Manages the hooked drivers - /// - Driver::HookedDriverManager DriverManager; - - /// - /// @brief Where all the intercepted IRPs are stored - /// - Driver::CapturedIrpManager IrpManager; - - - GlobalContext() : DriverObject {nullptr}, DeviceObject {nullptr}, Owner {nullptr}, ContextLock {}, SessionId(-1) - { - dbg("Creating GlobalContext"); - } - - - ~GlobalContext() - { - dbg("Destroying GlobalContext"); - DriverObject = nullptr; - DeviceObject = nullptr; - Owner = nullptr; - } - - static void* - operator new(usize sz) - { - void* Memory = ::ExAllocatePoolWithTag(NonPagedPoolNx, sz, CFB_DEVICE_TAG); - if ( Memory ) - { - dbg("Allocated GlobalContext at %p", Memory); - ::RtlSecureZeroMemory(Memory, sz); - } - return Memory; - } - - static void - operator delete(void* m) - { - dbg("Deallocating GlobalContext"); - ::ExFreePoolWithTag(m, CFB_DEVICE_TAG); - m = nullptr; - return; - } -}; - -/// -/// @brief Reference to the global driver context. -/// -extern struct GlobalContext* Globals; +#pragma once + +// clang-format off +#include "Common.hpp" +#include "DriverUtils.hpp" +#include "Log.hpp" + +#include "CapturedIrpManager.hpp" +#include "HookedDriverManager.hpp" +// clang-format on + + +#define CFB_MAX_HEXDUMP_BYTE 64 + +namespace Driver = CFB::Driver; +namespace Utils = CFB::Driver::Utils; + +struct GlobalContext +{ + /// + /// @brief Any critical read/write operation to the global context structure must acquire this lock. + /// + Utils::KQueuedSpinLock ContextLock; + + /// + /// @brief A pointer to the device object + /// + PDRIVER_OBJECT DriverObject; + + /// + /// @brief A pointer to the device object + /// + PDEVICE_OBJECT DeviceObject; + + /// + /// @brief A pointer to the EPROCESS of the broker. Not more than one handle to the + /// device is allowed. + /// + PEPROCESS Owner; + + /// + /// @brief Incremental session ID number. + /// + ULONG SessionId; + + /// + /// @brief Manages the hooked drivers + /// + Driver::HookedDriverManager DriverManager; + + /// + /// @brief Where all the intercepted IRPs are stored + /// + Driver::CapturedIrpManager IrpManager; + + + GlobalContext() : DriverObject {nullptr}, DeviceObject {nullptr}, Owner {nullptr}, ContextLock {}, SessionId(-1) + { + dbg("Creating GlobalContext"); + } + + + ~GlobalContext() + { + dbg("Destroying GlobalContext"); + DriverObject = nullptr; + DeviceObject = nullptr; + Owner = nullptr; + } + + static void* + operator new(usize sz) + { + void* Memory = ::ExAllocatePoolWithTag(NonPagedPoolNx, sz, CFB_DEVICE_TAG); + if ( Memory ) + { + dbg("Allocated GlobalContext at %p", Memory); + ::RtlSecureZeroMemory(Memory, sz); + } + return Memory; + } + + static void + operator delete(void* m) + { + dbg("Deallocating GlobalContext"); + ::ExFreePoolWithTag(m, CFB_DEVICE_TAG); + m = nullptr; + return; + } +}; + +/// +/// @brief Reference to the global driver context. +/// +extern struct GlobalContext* Globals; diff --git a/Driver/Source/CapturedIrp.cpp b/Driver/Source/CapturedIrp.cpp index 4f68e64..52edaf8 100644 --- a/Driver/Source/CapturedIrp.cpp +++ b/Driver/Source/CapturedIrp.cpp @@ -156,6 +156,7 @@ CapturedIrp::CapturePreCallData(_In_ PIRP Irp) return STATUS_UNSUCCESSFUL; } + NTSTATUS Status = STATUS_UNSUCCESSFUL; const ULONG Flags = m_DeviceObject->Flags; const PIO_STACK_LOCATION Stack = ::IoGetCurrentIrpStackLocation(Irp); @@ -164,8 +165,9 @@ CapturedIrp::CapturePreCallData(_In_ PIRP Irp) dbg("CapturePreCallData(%p)", Irp); - m_MajorFunction = Stack->MajorFunction; - m_MinorFunction = Stack->MinorFunction; + m_MajorFunction = Stack->MajorFunction; + m_MinorFunction = Stack->MinorFunction; + const ULONG Method = METHOD_FROM_CTL_CODE(m_IoctlCode); // // Determine & allocate the input/output buffer sizes from the IRP for "normal" IOCTLs @@ -198,74 +200,69 @@ CapturedIrp::CapturePreCallData(_In_ PIRP Irp) // // Now, copy the input/output buffer content to the CapturedIrp object // - NTSTATUS Status = STATUS_UNSUCCESSFUL; + PVOID Buffer = m_InputBuffer.get(); + ULONG BufferLength = m_InputBuffer.size(); - switch ( m_MajorFunction ) - { - case IRP_MJ_DEVICE_CONTROL: - case IRP_MJ_INTERNAL_DEVICE_CONTROL: + // __try // TODO fix unwinding { - // - // Handle NEITHER method for IOCTLs - // - if ( METHOD_FROM_CTL_CODE(m_IoctlCode) == METHOD_NEITHER ) + Status = STATUS_SUCCESS; + do { - if ( Stack->Parameters.DeviceIoControl.Type3InputBuffer < (PVOID)(1 << 16) ) + if ( (Stack->MajorFunction == IRP_MJ_DEVICE_CONTROL || + Stack->MajorFunction == IRP_MJ_INTERNAL_DEVICE_CONTROL) && + Method == METHOD_NEITHER ) { - return STATUS_INVALID_PARAMETER; + if ( Stack->Parameters.DeviceIoControl.Type3InputBuffer >= (PVOID)(1 << 16) ) + RtlCopyMemory(Buffer, Stack->Parameters.DeviceIoControl.Type3InputBuffer, BufferLength); + else + Status = STATUS_INVALID_PARAMETER; + break; } - ::memcpy(m_InputBuffer.get(), Stack->Parameters.DeviceIoControl.Type3InputBuffer, m_InputBuffer.size()); - Status = STATUS_SUCCESS; - } - break; - } - default: - { - // - // Otherwise, use copy method from flags - // - if ( Flags & DO_BUFFERED_IO ) - { - if ( !Irp->AssociatedIrp.SystemBuffer ) + if ( Method == METHOD_BUFFERED ) { - return STATUS_INVALID_PARAMETER_1; + if ( Irp->AssociatedIrp.SystemBuffer ) + RtlCopyMemory(Buffer, Irp->AssociatedIrp.SystemBuffer, BufferLength); + else + Status = STATUS_INVALID_PARAMETER_1; + break; } - ::memcpy(m_InputBuffer.get(), Irp->AssociatedIrp.SystemBuffer, m_InputBuffer.size()); - Status = STATUS_SUCCESS; - } - else if ( Flags & DO_DIRECT_IO ) - { - if ( !Irp->MdlAddress ) + if ( Method == METHOD_IN_DIRECT || Method == METHOD_OUT_DIRECT ) { - return STATUS_INVALID_PARAMETER_2; + if ( !Irp->MdlAddress ) + { + Status = STATUS_INVALID_PARAMETER_2; + break; + } + + PVOID pDataAddr = MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority); + if ( !pDataAddr ) + { + Status = STATUS_INVALID_PARAMETER_3; + break; + } + + RtlCopyMemory(Buffer, pDataAddr, BufferLength); } - - PVOID pDataAddr = ::MmGetSystemAddressForMdlSafe(Irp->MdlAddress, NormalPagePriority); - if ( !pDataAddr ) - { - return STATUS_INVALID_PARAMETER_3; - } - - ::memcpy(m_InputBuffer.get(), pDataAddr, m_InputBuffer.size()); - Status = STATUS_SUCCESS; - } - break; - } + } while ( 0 ); } + // __except (EXCEPTION_EXECUTE_HANDLER) + // { + // Status = GetExceptionCode(); + // } if ( !NT_SUCCESS(Status) ) { err("CapturePreCallData() returned %#08x", Status); } -#ifdef _DEBUG + // #ifdef _DEBUG else { dbg("Captured input data (%lu bytes):", m_InputBuffer.size()); CFB::Utils::Hexdump(m_InputBuffer.get(), MIN(m_InputBuffer.size(), CFB_MAX_HEXDUMP_BYTE)); } -#endif // _DEBUG + // #endif // _DEBUG return STATUS_SUCCESS; } @@ -336,10 +333,10 @@ CapturedIrp::CapturePostCallData(_In_ PIRP Irp, _In_ NTSTATUS ReturnedIoctlStatu // ::memcpy(m_OutputBuffer.get() + Offset, UserBuffer, Count); -#ifdef _DEBUG + // #ifdef _DEBUG dbg("Capturing output data:"); CFB::Utils::Hexdump(m_OutputBuffer.get(), MIN(m_OutputBuffer.size(), CFB_MAX_HEXDUMP_BYTE)); -#endif // _DEBUG + // #endif // _DEBUG return STATUS_SUCCESS; } diff --git a/Driver/Source/Entry.cpp b/Driver/Source/Entry.cpp index 87d0333..bec78e4 100644 --- a/Driver/Source/Entry.cpp +++ b/Driver/Source/Entry.cpp @@ -361,9 +361,9 @@ _Function_class_(DRIVER_DISPATCH) DriverReadRoutine(_In_ PDEVICE_OBJECT DeviceOb i + 1, DumpableIrpNumber, CurrentIrp->InputDataSize()); -#ifdef _DEBUG + // #ifdef _DEBUG CFB::Utils::Hexdump(Buffer, MIN(CurrentIrp->InputDataSize(), CFB_MAX_HEXDUMP_BYTE)); -#endif // _DEBUG + // #endif // _DEBUG } // @@ -385,9 +385,9 @@ _Function_class_(DRIVER_DISPATCH) DriverReadRoutine(_In_ PDEVICE_OBJECT DeviceOb i + 1, DumpableIrpNumber, CurrentIrp->OutputDataSize()); -#ifdef _DEBUG + // #ifdef _DEBUG CFB::Utils::Hexdump(Buffer, MIN(CurrentIrp->OutputDataSize(), CFB_MAX_HEXDUMP_BYTE)); -#endif // _DEBUG + // #endif // _DEBUG } dbg("IRP %d/%d returned to client", i + 1, DumpableIrpNumber); diff --git a/README.md b/README.md index ce21280..a3b55eb 100644 --- a/README.md +++ b/README.md @@ -11,12 +11,23 @@ ## What is it? -**Canadian Furious Beaver** is a distributed tool for capturing IRPs sent to any Windows driver. It operates in 2 parts: +> [!CAUTION] +> CFB is meant for research and debug purposes, it should never be used on production systems. Also BSoD may happen. You've been warned. + +**Canadian Furious Beaver** is a [`ProcMon`](https://learn.microsoft.com/en-us/sysinternals/downloads/procmon)-style tool designed only for capturing IRPs sent to any Windows driver. It operates in 2 parts: 1. the "Broker" combines both a user-land agent and a self-extractable driver (`IrpMonitor.sys`) that will install itself on the targeted system. After installing the driver, the broker will expose a TCP port listening (by default, on TCP/1337) and start collecting IRP from hooked drivers. The communication protocol was made to be simple by design (i.e. not secure) allowing any [3rd party tool](https://github.com/hugsy/cfb-cli) to dump the driver IRPs from the same Broker easily (via simple JSON messages). 2. the clients can connect to the broker, and will receive IRPs as a JSON message making it easy to view, or convert to another format. +![GUI](https://i.imgur.com/MUFYrL2.png) + +![CLI](https://i.imgur.com/5MWjqLa.png) + +3. IRPs (metadata, input/output buffers) can be stored to file on disk in the JSON format allowing for easy further scripting. + +> [!WARNING] +> Although the CFB driver (`IrpMonitor.sys`) should not violate patchguard, it however is only self-signed and so requires [`TestSigning`](https://learn.microsoft.com/en-us/windows-hardware/drivers/install/the-testsigning-boot-configuration-option) enabled in the BCD ## Why the name? @@ -25,5 +36,4 @@ Because I had no idea for the name of this tool, so it was graciously generated ## Kudos * `processhacker` for their [`phnt` header files](https://github.com/processhacker/phnt) - * `p-ranav` for their [`argparse` library](https://github.com/p-ranav/argparse) * `nlohmann` for their [`json` library](https://github.com/nlohmann/json)