Fixing TODOs (1/x) (#21)

* [cmake] add asan to public options of commonlib

* [ctf::remote] host can be passed as string

* [pwn::system] update pidof signature to take a copy string view  (not a reference)

* [pwn::win::process] completely restored all features and tests

* [ci/build] disable building docs

* fixed x86 builds

* added basic perf test for commonlib

.github/workflows/build.yml

@@ -15,7 +15,7 @@ jobs:
   build:
     name: "${{ matrix.variants.os }}/${{ matrix.variants.arch }}/${{ matrix.variants.config }}"
     env:
-      CMAKE_FLAGS: '-DPWN_BUILD_DOCS=ON -DPWN_DISASSEMBLE_X86=ON -DPWN_DISASSEMBLE_ARM64=ON -DPWN_BUILD_TOOLKIT=ON -DPWN_BUILD_TESTING=ON -DPWN_ENABLE_LUA_BACKDOOR=OFF'
+      CMAKE_FLAGS: '-DPWN_BUILD_DOCS=OFF -DPWN_DISASSEMBLE_X86=ON -DPWN_DISASSEMBLE_ARM64=ON -DPWN_BUILD_TOOLKIT=ON -DPWN_BUILD_TESTING=ON -DPWN_ENABLE_LUA_BACKDOOR=OFF'
       NB_CPU: 1
 
     strategy:

Modules/CTF/Include/CTF/Linux/Remote.hpp

@@ -21,6 +21,7 @@ class Remote : public Net::Tube
 {
 public:
     Remote(std::wstring_view const& host, const u16 port);
+    Remote(std::string_view const& host, const u16 port);
 
     ~Remote();
 

Modules/CTF/Source/Linux/Remote.cpp

@@ -29,6 +29,11 @@ CTF::Remote::Remote(std::wstring_view const& host, const u16 port) :
     }
 }
 
+CTF::Remote::Remote(std::string_view const& host, const u16 port) :
+    CTF::Remote::Remote(Utils::StringLib::To<std::wstring>(std::string(host)), port)
+{
+}
+
 CTF::Remote::~Remote()
 {
     Disconnect();
@@ -48,7 +53,6 @@ CTF::Remote::Connect()
     if ( ::connect(m_Socket, (struct sockaddr*)&sin, sizeof(sin)) < 0 )
     {
         ::perror("connect()");
-        Disconnect();
         return Err(ErrorCode::ExternalApiCallFailed);
     }
 
@@ -62,14 +66,16 @@ CTF::Remote::Disconnect()
 {
     auto res = true;
 
+    info(L"Closing socket {}", m_Socket);
+
     if ( ::close(m_Socket) < 0 )
     {
         ::perror("closesocket()");
-        res = false;
+        return Err(ErrorCode::ExternalApiCallFailed);
     }
 
-    dbg(L"session to {}:{} closed", m_Host.c_str(), m_Port);
-
+    m_Socket = 0;
+    info(L"Session to {}:{} closed", m_Host.c_str(), m_Port);
     return Ok(res);
 }
 
@@ -89,7 +95,6 @@ CTF::Remote::send_internal(_In_ std::vector<u8> const& out)
     if ( res < 0 )
     {
         ::perror("send()");
-        Disconnect();
         return Err(ErrorCode::ExternalApiCallFailed);
     }
 

Modules/Common/CMakeLists.txt

@@ -71,6 +71,14 @@ if(WIN32)
 
 else()
     target_compile_definitions(${PROJECT_NAME} PUBLIC PWN_BUILD_FOR_LINUX)
+
+    target_compile_options(${PROJECT_NAME}
+        PUBLIC
+        $<$<COMPILE_LANGUAGE:CXX>:
+        $<$<CONFIG:Debug>:-fsanitize=address>
+        >
+        PRIVATE
+    )
 endif(WIN32)
 
 install(DIRECTORY ${INTERFACE_DIR} DESTINATION ${CMAKE_PROJECT_NAME})

Modules/Common/Tests/CMakeLists.txt

@@ -1,21 +1,25 @@
 enable_testing()
-set(TEST_EXECUTABLE_NAME tests_pwn_${PROJECT_NAME})
 set(TEST_DIR ${CMAKE_CURRENT_LIST_DIR})
-list(APPEND SOURCE_FILES
+list(APPEND TEST_SOURCE_FILES
 
-    ${TEST_DIR}/main.cpp
-    ${TEST_DIR}/pwn_common.cpp
+    # Test cases
+    ${TEST_DIR}/tests_pwn_common.cpp
+
+    # Performance tests
+    ${TEST_DIR}/perf_pwn_common.cpp
 )
 
-add_executable(${TEST_EXECUTABLE_NAME} ${SOURCE_FILES})
-add_executable(PWN::Tests::${PROJECT_NAME} ALIAS ${TEST_EXECUTABLE_NAME})
-add_dependencies(${TEST_EXECUTABLE_NAME} PWN::Deps::Catch2 PWN::${PROJECT_NAME})
-target_link_libraries(${TEST_EXECUTABLE_NAME} PUBLIC PWN::Deps::Catch2 PWN::${PROJECT_NAME})
+foreach(SOURCE_FILE ${TEST_SOURCE_FILES})
+    cmake_path(GET SOURCE_FILE STEM TEST_EXECUTABLE_NAME)
+    add_executable(${TEST_EXECUTABLE_NAME} ${SOURCE_FILE})
+    add_dependencies(${TEST_EXECUTABLE_NAME} PWN::Deps::Catch2 PWN::${PROJECT_NAME})
+    target_link_libraries(${TEST_EXECUTABLE_NAME} PUBLIC PWN::Deps::Catch2 PWN::${PROJECT_NAME})
 
-if(WIN32)
-    target_link_options(${TEST_EXECUTABLE_NAME} PUBLIC /SUBSYSTEM:Console)
-endif(WIN32)
+    if(WIN32)
+        target_link_options(${TEST_EXECUTABLE_NAME} PUBLIC /SUBSYSTEM:Console)
+    endif(WIN32)
 
-add_test(NAME ${TEST_EXECUTABLE_NAME} COMMAND $<TARGET_FILE:${TEST_EXECUTABLE_NAME}>)
-set_tests_properties(${TEST_EXECUTABLE_NAME} PROPERTIES LABELS Common LABELS Version)
-install(FILES $<TARGET_FILE:${TEST_EXECUTABLE_NAME}> DESTINATION Tests)
+    add_test(NAME ${TEST_EXECUTABLE_NAME} COMMAND $<TARGET_FILE:${TEST_EXECUTABLE_NAME}>)
+    set_tests_properties(${TEST_EXECUTABLE_NAME} PROPERTIES LABELS Common LABELS Version)
+    install(FILES $<TARGET_FILE:${TEST_EXECUTABLE_NAME}> DESTINATION Tests)
+endforeach()

Modules/Common/Tests/perf_pwn_common.cpp

@@ -0,0 +1,75 @@
+#define CATCH_CONFIG_MAIN
+#define CATCH_CONFIG_ENABLE_BENCHMARKING
+
+#include <catch.hpp>
+
+#include "Common.hpp"
+
+Result<int>
+TestFunc11(uint32_t i)
+{
+    if ( i == 42 )
+    {
+        return Ok(1);
+    }
+
+    return Err(ErrorCode::RuntimeError);
+}
+
+int
+TestFunc12(uint32_t i)
+{
+    if ( i == 42 )
+    {
+        return 1;
+    }
+
+    return -1;
+}
+
+
+Result<std::vector<int>>
+TestFunc21(uint32_t i)
+{
+    if ( i == 42 )
+    {
+        return Ok(std::move(std::vector<int> {1, 2, 3}));
+    }
+
+    return Err(ErrorCode::RuntimeError);
+}
+
+std::vector<int>
+TestFunc22(uint32_t i)
+{
+    if ( i == 42 )
+    {
+        return std::vector<int> {1, 2, 3};
+    }
+
+    return std::vector<int> {};
+}
+
+
+TEST_CASE("Simple perf checks", "Benchmarks::Error")
+{
+    BENCHMARK("[managed] basic return type - success")
+    {
+        return std::move(TestFunc11(42));
+    };
+
+    BENCHMARK("[unmanaged] basic return type - success")
+    {
+        return TestFunc12(42); // prevent optimization
+    };
+
+    BENCHMARK("[managed] basic return type - failure")
+    {
+        return std::move(TestFunc11(1));
+    };
+
+    BENCHMARK("[unmanaged] basic return type - failure")
+    {
+        return TestFunc12(1);
+    };
+}

Modules/Common/Tests/pwn_common.cpp → Modules/Common/Tests/tests_pwn_common.cpp

@@ -1,3 +1,5 @@
+#define CATCH_CONFIG_MAIN
+
 #include <catch.hpp>
 
 #include "Common.hpp"

Modules/Process/Include/Win32/Process.hpp

@@ -29,34 +29,73 @@ class Thread;
 class Memory
 {
 public:
+    ///
+    ///@brief Default constructor
+    ///
     Memory() = default;
 
 
-    Memory(Process const& _Process) : m_Process {_Process}
-    {
-    }
+    ///
+    ///@brief Construct a new Memory object for a given process
+    ///
+    ///@param _Process
+    ///
+    Memory(Process& _Process);
 
 
+    ///
+    ///@brief Read some bytes from memory
+    ///
+    ///@param Address
+    ///@param Length
+    ///@return Result<std::vector<u8>>
+    ///
     auto
     Read(uptr const Address, usize Length) -> Result<std::vector<u8>>;
 
 
+    ///
+    ///@brief Write some bytes to memory
+    ///
+    ///@param Address
+    ///@param data
+    ///@return Result<usize>
+    ///
     auto
     Write(uptr const Address, std::vector<u8> data) -> Result<usize>;
 
 
+    ///
+    ///@brief Fill the memory with specific byte
+    ///
+    ///@param address
+    ///@param size
+    ///@param val
+    ///@return Result<usize>
+    ///
     auto
     Memset(uptr const address, const usize size, const u8 val = 0x00) -> Result<usize>;
 
+
+    ///
+    ///@brief Allocate memory in the local/remote process
+    ///
+    ///@param Size
+    ///@param Permission
+    ///@param ForcedMappingAddress
+    ///@param wipe
+    ///@return Result<uptr>
+    ///
     auto
     Allocate(
         const size_t Size,
         const wchar_t Permission[3]     = L"rwx",
         const uptr ForcedMappingAddress = 0,
         bool wipe                       = true) -> Result<uptr>;
 
-    auto
-    Free(const uptr Address) -> bool;
+
+    Result<bool>
+    Free(const uptr Address);
 
 
     ///
@@ -67,7 +106,7 @@ class Memory
     ///@return Result<std::shared_ptr<T>>
     ///
     template<class T>
-    Result<std::shared_ptr<T>>
+    Result<std::unique_ptr<T>>
     Query(const MEMORY_INFORMATION_CLASS MemoryInformationClass, const uptr BaseAddress = nullptr)
     {
         auto res = QueryInternal(MemoryInformationClass, BaseAddress, sizeof(T));
@@ -76,20 +115,17 @@ class Memory
             return Error(res);
         }
 
-        const auto p = reinterpret_cast<T*>(Value(res));
-        auto deleter = [](T* x)
-        {
-            ::LocalFree(x);
-        };
-        return Ok(std::shared_ptr<T>(p, deleter));
+        auto RawResult = Value(std::move(res));
+        std::unique_ptr<T> TypedResult {(T*)RawResult.release()};
+        return Ok(std::move(TypedResult));
     }
 
     ///
     ///@brief
     ///
     ///@return Result < std::vector < MEMORY_BASIC_INFORMATION>>
     ///
-    Result<std::vector<std::shared_ptr<MEMORY_BASIC_INFORMATION>>>
+    Result<std::vector<std::unique_ptr<MEMORY_BASIC_INFORMATION>>>
     Regions();
 
 
@@ -109,14 +145,14 @@ class Memory
     ///
     /// @param ProcessInformationClass
     ///
-    /// @return Result<PVOID>
+    /// @return Result<std::unique_ptr<u8[]>>
     ///
-    Result<PVOID>
+    Result<std::unique_ptr<u8[]>>
     QueryInternal(const MEMORY_INFORMATION_CLASS, const uptr BaseAddress, const usize);
 
 
 private:
-    Process const& m_Process;
+    Process& m_Process;
 };
 
 
@@ -215,10 +251,11 @@ class Process
         return m_ParentProcessId;
     }
 
+
     Result<pwn::Process::Process>
     Parent()
     {
-        if ( m_ParentProcessId < 0 )
+        if ( m_ParentProcessId <= 0 )
         {
             return Err(ErrorCode::InvalidProcess);
         }
@@ -231,10 +268,10 @@ class Process
     ///
     ///@return fs::path const&
     ///
-    std::filesystem::path const&
+    std::wstring const&
     Path() const
     {
-        return m_Path;
+        return m_NativePath;
     }
 
 
@@ -318,7 +355,7 @@ class Process
     /// @brief Query process information
     ///
     /// @param ProcessInformationClass
-    /// @return Result<std::shared_ptr<T>>
+    /// @return Result<std::unique_ptr<T>>
     ///
     template<class T>
     Result<std::unique_ptr<T>>
@@ -445,7 +482,7 @@ class Process
 private: // Members
     u32 m_ProcessId {0};
     u32 m_ParentProcessId {0};
-    std::filesystem::path m_Path {};
+    std::wstring m_NativePath {};
     Integrity m_IntegrityLevel {Integrity::Unknown};
     UniqueHandle m_ProcessHandle {nullptr};
     DWORD m_ProcessHandleAccessMask {0};