Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple bugs in the driver's implementation #12

Open
repnz opened this issue May 8, 2019 · 0 comments
Open

Multiple bugs in the driver's implementation #12

repnz opened this issue May 8, 2019 · 0 comments

Comments

@repnz
Copy link

repnz commented May 8, 2019

I know it's a POC, but I thought it would be good to fix this anyway -

  1. There is a stack buffer overflow reading the 'pid' from user mode. Replace inBufferLength with sizeof(pid).

    strcpy_s(pid, inBufferLength, inBuf);

  2. The output buffer's length is not checked

    RtlCopyBytes(buffer, data, outBufferLength);

  3. This memory is not freed anywhere

    LPSTR result = ExAllocatePool(NonPagedPool, sizeof(ULONG) + 20);;

  4. This buffer was allocated with length=(sizeof(ULONG) + 20), why is the param to sprintf_s longer?

    sprintf_s(result, 2 * sizeof(ULONG) + 30, "Found offsets: %lu & %lu", PID_OFFSET, LIST_OFFSET);

  5. This string is not used anywhere (copied from the microsoft ioctl sample:) )

    PCHAR data = "This String is from Device Driver !!!";

  6. Here, you use 'datalen' which is the length of the string from the sample instead of the real result string:

    Irp->IoStatus.Information = (outBufferLength<datalen ? outBufferLength : datalen);

@repnz repnz changed the title Buffer Overflow Reading "pid" from user mode Multipile buffer overflows May 8, 2019
@repnz repnz changed the title Multipile buffer overflows Multiple bugs in the driver's implementation May 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant