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

Stack overflow secure version strange size #27

Closed benoitsevens closed 5 years ago

benoitsevens commented 5 years ago

Hello,

I don't think this is very important but just a very little detail.

In the secure version of Driver/Stackoverflow.c wouldn't it be more logical to replace:

RtlCopyMemory((PVOID)KernelBuffer, UserBuffer, sizeof(KernelBuffer));

with:

RtlCopyMemory((PVOID)KernelBuffer, UserBuffer, min(sizeof(KernelBuffer), Size));

In the case that DeviceIoControl provides a buffer with Size smaller than sizeof(KernelBuffer), the driver will copy garbage to its stack, or maybe trigger an exception if that memory is not mapped in user land. I don't think this is exploitable (so it does not make it UNSECURE), but it just seems not very logical.

Regards

hacksysteam commented 5 years ago

Hi @b3n7s

This is a known issue and there is nothing much we can do for this.

The solution that you are proposing is having the same issue as the current implementation.

Even if we pick min(), then also it doesn't give guarantee that the pages would be mapped.

However, there is no issue copying the junk data in the kernel stack buffer because it's not being used in some critical operation.

Also, not that if the page is not mapped then there would not be crash because ProbeForRead would find this and throw a exception which will be handled by try/catch

Thank you.

hacksysteam commented 5 years ago

@b3n7s is this issue good to be closed?

benoitsevens commented 5 years ago

@hacksysteam Of course. Sorry for the late reply :)