Explicitly pass the size of the requested arguments

[1ndahous3]

Due to refactoring in #155 the argument size bug was introduced.

Initially the code was like this:

  // ntdll!NtDeviceIoControlFile()
  // Ioctl:              @rsp + 0x30
  // InputBuffer:        @rsp + 0x38
  // InputBufferLength:  @rsp + 0x40

  const Gva_t Stack = Gva_t(g_Backend->Rsp());

  const Gva_t InputBufferLengthPtr = Stack + Gva_t(0x40);
  uint32_t CurrentIoctlBufferSize = g_Backend->VirtRead4(InputBufferLengthPtr);

But after refactoring it became like this:

wtf/src/wtf/fuzzer_ioctl.cc

Lines 77 to 89 in 9823579

	const auto &[InputBufferSize, InputBufferSizePtr] =
	g_Backend->GetArgAndAddress(7);
	const uint32_t MutatedInputBufferSize =
	std::min(TotalInputBufferSize, uint32_t(InputBufferSize));
	//
	// Calculate the new InputBuffer address by pushing the mutated buffer as
	// close as possible from its end.
	//
	const auto &[InputBuffer, InputBufferPtr] = g_Backend->GetArgAndAddress(6);
	const auto NewInputBuffer =
	Gva_t(InputBuffer + InputBufferSize - MutatedInputBufferSize);

So 8 bytes are read for the InputBufferSize argument, and we got an invalid value - only one uint32_t(InputBufferSize) cast is made, but then the same cast is not made during addition operation.

This patch replaces one argument getter function with two: GetArg4() for 4-byte values and GetArg8() for 8-byte values. Also, many additional casts to uint32_t have been removed, which are no longer needed.

[0vercl0k]

Oops! Thanks for the PR - I am currently traveling so will take a look at this in December; I hope it's ok! Cheers

…

On Sat, Nov 4, 2023 at 3:46 AM Roman ***@***.***> wrote: Due to refactoring in #155 <#155> the argument size bug was introduced. Initially the code was like this: https://github.com/1ndahous3/wtf/blob/a874ebaf71da4f94df1dba85fb3ca18441e96f42/src/wtf/fuzzer_ioctl.cc#L48-L56 But after refactoring it became like this: https://github.com/0vercl0k/wtf/blob/9823579ef764b0b3c0af2f71b61d5aa47fb3de51/src/wtf/fuzzer_ioctl.cc#L77-L89 So 8 bytes are read for the InputBufferSize argument, and we got an invalid value - only one uint32_t(InputBufferSize) cast is made, but then the same cast is not made during addition operation. This patch replaces one argument getter function with two: GetArg4() for 4-byte values and GetArg8() for 8-byte values. Also, many additional casts to uint32_t have been removed, which are no longer needed. ------------------------------ You can view, comment on, or merge this pull request online at: #188 Commit Summary - def27ff <def27ff> explicitly pass the size of the requested arguments File Changes (5 files <https://github.com/0vercl0k/wtf/pull/188/files>) - *M* src/wtf/backend.cc <https://github.com/0vercl0k/wtf/pull/188/files#diff-9bdcbde23d1a3d31bcf365bc3cd732076631d9052bcce45511ed2f1f3cddf6a0> (28) - *M* src/wtf/backend.h <https://github.com/0vercl0k/wtf/pull/188/files#diff-4ea7b0abd234b1b9780f5c380cb247f80b3623b058ec68179d26687fa6598345> (6) - *M* src/wtf/fshooks.cc <https://github.com/0vercl0k/wtf/pull/188/files#diff-12fd18be26aaa7db94d9efb321e2de95ed90d7d96e5f8088f3c49acf48d7e9b0> (68) - *M* src/wtf/fuzzer_hevd.cc <https://github.com/0vercl0k/wtf/pull/188/files#diff-9d112d854dfe9b0c57ae252aec0e7adb30f02864a29837cc4a7ab48d24d0fa24> (12) - *M* src/wtf/fuzzer_ioctl.cc <https://github.com/0vercl0k/wtf/pull/188/files#diff-b9ef863d266837d9694d2f95e16f1a9c4b86e26c3951c0169df9268b8bb1d30e> (18) Patch Links: - https://github.com/0vercl0k/wtf/pull/188.patch - https://github.com/0vercl0k/wtf/pull/188.diff — Reply to this email directly, view it on GitHub <#188>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALIORP4VZ2PMYNL5NFNDBLYCWUCFAVCNFSM6AAAAAA65IBC3GVHI2DSMVQWIX3LMV43ASLTON2WKOZRHE3TOMJTGA3DGMA> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[0vercl0k]

Looking into this..

[0vercl0k]

@1ndahous3 what do you think of the new changes?

Cheers

[1ndahous3]

@0vercl0k looks good, I think it makes no difference for GetArg4() to read 4 bytes or read 8 bytes with garbage and trim it through a uint32_t cast.

In my opinion, it would be nice to rename GetArg() to GetArg8() to show the size not only by the type of the return value. This will not give a chance to make a bug when writing code without noticing the GetArg4() pair function.

[0vercl0k]

I do agree, but it also means it breaks everybody that is using GetArg which is not desirable. What I can do is to introduce a GetArg8 version though to be consistent - wdyt?

Cheers

[1ndahous3]

What I can do is to introduce a GetArg8 version though to be consistent - wdyt?

Sounds like a good compromise.

[0vercl0k]

All right - take this for a spin and let me know 🫡

Cheers

[1ndahous3]

AFAIR the BugCheckCode from KeBugCheck2() is of ULONG type, so we need to get the uint32_t value.

[0vercl0k]

Ha yeah I forgot to update it - will do that tonight! Cheers

…

On Tue, Nov 28, 2023 at 7:03 AM Roman ***@***.***> wrote: AFAIR the BugCheckCode from KeBugCheck2() is of ULONG type, so we need to get the uint32_t value. — Reply to this email directly, view it on GitHub <#188 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALIORLO6RYMMOPFJDTN44LYGV5BZAVCNFSM6AAAAAA65IBC3GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRZGE3TINRRGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[0vercl0k]

Done!

[1ndahous3]

I can only suggest adding a [[deprecated]] attribute to the GetArg() function.

The code is ok.

[0vercl0k]

Neat, TIL [[deprecated]]!

[1ndahous3]

I don't have any additional suggestions or notes, everything looks great!

[0vercl0k]

All right, will merge this today - thank you! Cheers

…

On Fri, Dec 1, 2023 at 11:01 PM Roman ***@***.***> wrote: I don't have any additional suggestions or notes, everything looks great! — Reply to this email directly, view it on GitHub <#188 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALIORL4SOCEFD2MI43DEE3YHJHSVAVCNFSM6AAAAAA65IBC3GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZWHAZTMMBUGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>