hacksysteam / HackSysExtremeVulnerableDriver

HackSys Extreme Vulnerable Driver (HEVD) - Windows & Linux
https://hacksys.io
GNU General Public License v3.0
2.42k stars 525 forks source link

Fix Size type in IntegerOverflow #39

Closed takubokudori closed 4 years ago

takubokudori commented 4 years ago

SIZE_T is defined as ULONG_PTR. ULONG_PTR is defined as unsigned long on x86, but as unsigned __int64 on x64. Also InputBufferLength is defined as ULONG(unsigned long) on x86 and x64.

So when InputBufferLength is copied to Size in a driver built on x64, an implicit type conversion occurs and it becomes unsigned __int64 type, so even if InputBufferLength is passed 0xFFFFFFFF, it is still an integer overflow will not occur.

This pull request is to fix the issue.

hacksysteam commented 4 years ago

Hi @takubokudori

Thank you so much for the report.

SIZE_T is defined as ULONG_PTR. ULONG_PTR is defined as unsigned long on x86, but as unsigned __int64 on x64. Also InputBufferLength is defined as ULONG(unsigned long) on x86 and x64.

Correct

So when InputBufferLength is copied to Size in a driver built on x64, an implicit type conversion occurs and it becomes unsigned __int64 type, so even if InputBufferLength is passed 0xFFFFFFFF, it is still an integer overflow will not occur.

This is also correct. However, what will happen when we pass 0xFFFFFFFFFFFFFFFF as the InputBufferLength? The overflow still will happen and this was how the challenge was designed. Yeah, I need to update the comments to reflect x64 as well.

What do you think? Do let me your thoughts.

takubokudori commented 4 years ago

Thank you for your reply.

The type of nInBufferSize of DeviceIoControl (and NtDeviceIoControl) is also defined as DWORD(unsigned long). So even if you pass 0xFFFFFFFFFFFFFFFF to nInBufferSize, a cast conversion from unsigned long long to unsigned long occurs, and eventually 0xFFFFFFFF will be passed to IrpSp->Parameters.DeviceIoControl.InputBufferLength (this is also ULONG, so it can store up to 0xFFFFFFFF), so BSOD will not occur.

I confirmed that the following program (and HEVDExploit.exe with 0xFFFFFFFFFFFFFFFF) didn’t cause BSOD before applying this patch and that BSOD occurred after this patch:

#include <stdio.h>
#include <windows.h>

#define HACKSYS_EVD_IOCTL_INTEGER_OVERFLOW CTL_CODE(FILE_DEVICE_UNKNOWN, 0x809, METHOD_NEITHER, FILE_ANY_ACCESS)

int main(void)
{
    HANDLE hFile = CreateFile(L"\\\\.\\HackSysExtremeVulnerableDriver",
        GENERIC_READ | GENERIC_WRITE,
        FILE_SHARE_READ | FILE_SHARE_WRITE,
        NULL,
        OPEN_EXISTING,
        FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OVERLAPPED,
        NULL);
    if (hFile == INVALID_HANDLE_VALUE)
    {
        printf("Failed to CreateFile\n");
        return 1;
    }
    char in_buf[1000] = { 0 };
    DWORD a;
    const unsigned long long x = 0xFFFFFFFFFFFFFFFF;
    DeviceIoControl(hFile,
        HACKSYS_EVD_IOCTL_INTEGER_OVERFLOW,
        (LPVOID)in_buf,
        x,
        NULL,
        0,
        &a,
        NULL);
    return 0;
}
hacksysteam commented 4 years ago

@takubokudori yes, I verified and you are correct. Bummer, I didn't consider that InputBufferLength is DWORD.

Thank you so much for the verification and explanation.

Very nice.