Code review pierwszego wiekszego projektu w c++ ;)

1

Udalo sie skonczyc bota do metka ;)

  • wallhack (ignorowanie kolizji z otoczeniem)
  • speedhack (na chodzenie)
  • autoattack (przydante gdy chcemy zostawic postac na expowisku i odejsc od kompa)
  • target damage (wysyla pakiety ataku na konkretny obiekt, co sprawia, ze hity sa kilkukrotnie mnozone wiec zabijanie bosow to sekundy)

Wrzucam pod review, czy kod jest w miare czytelny i czy ew cos poprawic pod katem "dobrych nawykow" w C.

#include <vector>
#include <windows.h>
#include "pch.h"
namespace H{
    
    struct clicks {
        bool wallhack_clicked = FALSE;
        bool autoAttack_clicked = FALSE;
        bool speedhack_clicked = FALSE;
        int normal_speed = 0;
    }click;

    class find_adress {
    protected:
        uintptr_t FinalAddress(uintptr_t ptr, std::vector<DWORD> offsets)
        {
            uintptr_t addr = ptr;
            for (unsigned int i = 0; i < offsets.size(); ++i)
            {
                addr = *(uintptr_t*)addr;
                addr += offsets[i];
            }
            return addr; // returns the main pointer from needed assets
        }
    };

    class hacks: public find_adress {
    private:
        uintptr_t module;
        void main_func(int value, uintptr_t adress, std::vector<DWORD> offsets) {
            uintptr_t adres = FinalAddress(adress, offsets);
            int* main_value = (int*)adres;
            *main_value = value;
        }
        bool SendAttack(int vid) {
            DWORD netPointer = *(DWORD*)0x06733F8;
            DWORD battleCall = 0x04B53A0;
            _asm {
                mov ecx, netPointer
                push vid
                push 0
                call battleCall
            }
        }
    public:
        hacks(uintptr_t mod) : module(mod) {};
        void wallHack(int value);
        void speedhack(int value);
        void attack(int value);
        void target();
    };
}
#include "pch.h"
#include <iostream>
#include "hacks.h"
using namespace H;

void hacks::wallHack(int value) {
    main_func(value, module + 0x267D94, { 0xC, 0x66C });
}
void hacks::speedhack(int value) {
    main_func(value, module + 0x267D94, { 0xC, 0x5a9 + 1 });
}
void hacks::attack(int value) {
    main_func(value, 0x0667D90, { 0x64 });
}
void hacks::target() {
    while (true) {
        if (GetAsyncKeyState(VK_F3) & 1) {
            break;
        }
        DWORD getTargetId = (*(DWORD*)(*(DWORD*)0x0667D90 + 0x0010FC4));
        SendAttack(getTargetId);
        Sleep(160);
    }
}
DWORD WINAPI HackThread(HMODULE hModule)
{
    AllocConsole();
    FILE* f;
    freopen_s(&f, "CONOUT$", "w", stdout);

    std::cout << "Cheats injected\n" << std::endl;

    uintptr_t moduleBase = (uintptr_t)GetModuleHandle(L"Soria2.pl.exe");
    hacks Hack(moduleBase);
    
    while (true)
    {
        if (GetAsyncKeyState(VK_F1) & 1)
        {
            if (click.wallhack_clicked) {
                click.wallhack_clicked = FALSE;
                Hack.wallHack(0);
            }
            else {
                click.wallhack_clicked = TRUE;
                Hack.wallHack(1);
            }
        }
        if (GetAsyncKeyState(VK_F2) & 1)
        {
            if (click.autoAttack_clicked) {
                click.autoAttack_clicked = FALSE;
                Hack.attack(0);
            }
            else {
                click.autoAttack_clicked = TRUE;
                Hack.attack(1);
            }
        }
        if (GetAsyncKeyState(VK_F3) & 1) {
            Hack.target();
        }
        if (GetAsyncKeyState(VK_F4) & 1) {
            if (click.speedhack_clicked) {
                click.speedhack_clicked = FALSE;
                Hack.speedhack(click.normal_speed);
            }
            else {
                click.speedhack_clicked = TRUE;
                Hack.speedhack(16500);
            }
        }
        Sleep(10);
    }
    fclose(f);
    FreeConsole();
    FreeLibraryAndExitThread(hModule, 0);
    return 0;
}

BOOL APIENTRY DllMain(HMODULE hModule,
    DWORD  ul_reason_for_call,
    LPVOID lpReserved
)
{
    switch (ul_reason_for_call)
    {
    case DLL_PROCESS_ATTACH:
    {
        CloseHandle(CreateThread(nullptr, 0, (LPTHREAD_START_ROUTINE)HackThread, hModule, 0, nullptr));
    }
    case DLL_THREAD_ATTACH:
    case DLL_THREAD_DETACH:
    case DLL_PROCESS_DETACH:
        break;
    }
    return TRUE;
}
2

Nie znam się na c++, ale:

  1. Soria2.pl.exe? To jest cheat do jakiegoś konkretnego klienta gry?
  2. Powynosiłbym adresy pamięci do jakiejś struktury. Często jest tak, że po aktualizacji klienta trzeba je również zaktualizować
4

Trochę mi nie pasuje określenie "większy projekt". które zawarłeś w tytule wątku do tego, co w nim zamieściłeś. bo wrzucony kod ma jakieś 150 linii, z czego jeszcze duża część (tak oceniam że z 20%) to jest boilerplate, odstępy pionowe między funkcjami, jednoznakowe linie np. z nawiasem klamrowym zamykającym blok itp.

Ale żeby nie było, że się tylko czepiam - ogólnie kod wygląda ładnie. Poniżej kilka uwag/przemyśleń:

  • stosujesz wcięcia (to jest dobre)
  • nazwy po angielsku - to dobry zwyczaj
  • 0x267D94, { 0xC, 0x66C } - zamiast stosować tzw. magiczne liczby (ang. magic numbers - https://en.wikipedia.org/wiki/Magicnumber(programming) ) lepiej, jakbyś te wartości opakował w jakieś zmienne/stałe i potem je podstawiał do wywołań funkcji. Zauważ, że jeśli pojawi się teraz konieczności zmienienia 0x66C na 0x60C to będziesz musiał znaleźć wszystkie miejsca w kodzie, w których ta wartość się pojawia. Poza tym osoby czytające Twój kod nie mają pojęcia, co ta wartość oznacza. Jakbyś ją nazwał jakoś w stylu memoryOffset to każdy by od razu wiedział, o co chodzi w tym fragmencie. To samo dotyczy także innych fragmentów - chociażby Hack.speedhack(16500); - tutaj lepiej by było to napisać jakoś w stylu Hack.speedhack(maxSpeed);, a wcześniej zdefiniować, że zmienna maxSpeed ma wartość 16500.
  • case DLL_THREAD_ATTACH: case DLL_THREAD_DETACH: case DLL_PROCESS_DETACH: - czemu to pozostawiłeś puste? Planujesz tam coś dodać w przyszłości, czy może coś było i wyleciało? Jeśli jest to na przyszłość, to możesz to na razie zakomentować (będzie wiadomo, że nie jest to przeoczenie/gdzieś kod uciekł, tylko celowo jest to fragment wyłączony z użycia), a jeśli już jest niepotrzebne, to w ogóle wywal,
3
  1. wszystkie wartości typu 0x04B53A0 powinny być jakimiś stałymi, których nazwy by coś mówiły
  2. linia 24 plik pierwszy: return addr; // returns the main pointer from needed assets : zamiast komentarza można by dać lepszą nazwę zmiennej
  3. funkcja HackThread moim zdaniem jest za długa. To co jest w ciele każdego "if'a" mogło by być oddzielną funkcją o nazwie, która by tłumaczyła ten kod
  4. w kilku miejscach przekazujesz cały vector. Może lepiej wykorzystać const referencję?
  5. "Soria2.pl.exe" - takie rzeczy powinny być stałymi
  6. Mam też wrażenie, że nie do końca dobrze wykorzystałeś obiektowość. Wydaje mi się, że funkcja HackThread mogłaby być metodą publiczną w klasie hacks, a wtedy pozostałe metody mogły by być prywatne. No i nie wiem po co to dziedziczenie.
2

Te hardkodowane adresy to słaby pomysł. Jakakolwiek zmiana w kliencie gry sprawi ze twój hack przestanie działać bo adresy się zmienią. Jeśli binarki maja symbole, to dużo sensowniej byłoby odczytać adresy jako jakiś offset od znanego symbolu - większa szansa że się szybko nie zepsuje, a nawet jeśli to wiadomo co to był za adres i jak to naprawić.

1 użytkowników online, w tym zalogowanych: 0, gości: 1