conhost.exe heap corruption in COOKED_READ_DATA::_handlePostCharInputLoop #13284

@emmett-b commented on GitHub (Apr 2, 2021):

I can provide minidumps with heap memory.
Here's the call stack using app verifier:

Exception thrown at 0x00007FFA7C66E35B (ucrtbase.dll) in conhost.exe: 0xC0000005: Access violation writing location 0x0000023781939290.

@emmett-b commented on GitHub (Apr 2, 2021): I can provide minidumps with heap memory. Here's the call stack using app verifier: ![image](https://user-images.githubusercontent.com/28929018/113393444-fafa0980-9396-11eb-9f06-10418dd55624.png) Exception thrown at 0x00007FFA7C66E35B (ucrtbase.dll) in conhost.exe: 0xC0000005: Access violation writing location 0x0000023781939290.

claunia commented

@emmett-b commented on GitHub (Apr 2, 2021):

@emmett-b commented on GitHub (Apr 2, 2021): ![image](https://user-images.githubusercontent.com/28929018/113393671-46acb300-9397-11eb-83c0-d5f6e2acbb84.png)

claunia commented

@emmett-b commented on GitHub (Apr 2, 2021):

Where could I post a minidump of the crash? Here it says "We don't support that file type". The dump is 155MB.

@emmett-b commented on GitHub (Apr 2, 2021): Where could I post a minidump of the crash? Here it says "We don't support that file type". The dump is 155MB.

claunia commented

@emmett-b commented on GitHub (Apr 2, 2021):

I tried adding locks to serialize my api calls but didn't seem to help.

@emmett-b commented on GitHub (Apr 2, 2021): I tried adding locks to serialize my api calls but didn't seem to help.

claunia commented

@WSLUser commented on GitHub (Apr 2, 2021):

Your best bet is to do a dump with feedback hub and post the link here. Also try zipping your dump.

@WSLUser commented on GitHub (Apr 2, 2021): Your best bet is to do a dump with feedback hub and post the link here. Also try zipping your dump.

claunia commented

@zadjii-msft commented on GitHub (Apr 2, 2021):

Your best bet is to do a dump with feedback hub

Meh, the Feedback Hub is garbage. If GitHub doesn't let you upload the .zip, then just email it to the address in my profile 😉 (and @ me here so I know to look for it)

@zadjii-msft commented on GitHub (Apr 2, 2021): > Your best bet is to do a dump with feedback hub Meh, the Feedback Hub is garbage. If GitHub doesn't let you upload the `.zip`, then just email it to the address in my profile 😉 (and @ me here so I know to look for it)

claunia commented

@emmett-b commented on GitHub (Apr 2, 2021):

@zadjii-msft just send you the zip file

@emmett-b commented on GitHub (Apr 2, 2021): @zadjii-msft just send you the zip file

claunia commented

@DHowett commented on GitHub (Apr 2, 2021):

That's not good! Thanks!

@DHowett commented on GitHub (Apr 2, 2021): _That's not good!_ Thanks!

claunia commented

@zadjii-msft commented on GitHub (Apr 2, 2021):

@ future self: I downloaded the zip to my corporate OneDrive. I'll probably take a look when I get around to a full triage pass Tuesday

@zadjii-msft commented on GitHub (Apr 2, 2021): @ future self: I downloaded the zip to my corporate OneDrive. I'll probably take a look when I get around to a full triage pass Tuesday

claunia commented

4b7d955012/src/host/readDataCooked.cpp (L1003)

@DHowett commented on GitHub (Apr 2, 2021):

_handlePostCharInputLoop is one of the our large, refactoring-resistant functions. It's older than the rest of the code, like WriteCharsLegacy, and critically important that we get right because it underpins the line editing behavior of CMD and everything else that uses cooked input.

It's here, for those interested:

@DHowett commented on GitHub (Apr 2, 2021): `_handlePostCharInputLoop` is one of the our large, refactoring-resistant functions. It's older than the rest of the code, like WriteCharsLegacy, and *critically important* that we get right because it underpins the line editing behavior of CMD and everything else that uses cooked input. It's here, for those interested: https://github.com/microsoft/terminal/blob/4b7d9550129615e14501abddbd92b04b33a4a0fb/src/host/readDataCooked.cpp#L1003

claunia commented

@emmett-b commented on GitHub (Apr 6, 2021):

I build OpenConsole in debug mode at commit 507a84664e (tag: v1.6.10571.0).
I YOLO'd my system by briefly replacing system32\conhost.exe.
Still using app verifier, crash is happening at readDataCooked.cpp:1158
memmove(_userBuffer, _backupLimit, numBytes);
_userBuffer points to non existent memory. So it seems _handlePostCharInputLoop isn't the culprit, _userBuffer is already wrong when it's called. Looks like a use-after-free.

@emmett-b commented on GitHub (Apr 6, 2021): I build `OpenConsole` in debug mode at commit 507a84664e6d5449bc31746510128d3136ecc719 (tag: v1.6.10571.0). I YOLO'd my system by briefly replacing system32\conhost.exe. Still using app verifier, crash is happening at readDataCooked.cpp:1158 `memmove(_userBuffer, _backupLimit, numBytes);` `_userBuffer` points to non existent memory. So it seems `_handlePostCharInputLoop` isn't the culprit, _userBuffer is already wrong when it's called. Looks like a use-after-free.

claunia commented

@emmett-b commented on GitHub (Apr 6, 2021):

On the crashing call stack we see that InputBuffer::Write calls WakeUpReadersWaitingForData (which then calls ConsoleWaitQueue::NotifyWaiters)
InputBuffer::WaitQueue contains 1 ConsoleWaitBlock item.
This wait block has _WaitReplyMessage.State.OutputBuffer == _pWaiter->_userBuffer pointing to non existent memory.
This waiting message probably had its output buffer freed somehow.
The waiting block api number is 0x01000005 (API_NUMBER_READCONSOLE).
So it has been inserted by a previous ReadConsoleOutput call.
The buffer must have been allocated in ApiDispatchers::ServerReadConsole, with a call to m->GetAugmentedOutputBuffer.
Not sure who is responsible for freeing this buffer but I guess it's freed prematurely.
I hope this helps.

@emmett-b commented on GitHub (Apr 6, 2021): On the crashing call stack we see that `InputBuffer::Write` calls `WakeUpReadersWaitingForData` (which then calls ` ConsoleWaitQueue::NotifyWaiters`) `InputBuffer::WaitQueue` contains 1 `ConsoleWaitBlock` item. This wait block has `_WaitReplyMessage.State.OutputBuffer` == `_pWaiter->_userBuffer` pointing to non existent memory. This waiting message probably had its output buffer freed somehow. The waiting block api number is 0x01000005 (API_NUMBER_READCONSOLE). So it has been inserted by a previous `ReadConsoleOutput` call. The buffer must have been allocated in `ApiDispatchers::ServerReadConsole`, with a call to `m->GetAugmentedOutputBuffer`. Not sure who is responsible for freeing this buffer but I guess it's freed prematurely. I hope this helps.

claunia commented

@emmett-b commented on GitHub (Apr 6, 2021):

@zadjii-msft I can send you a debug mode minidump of the crash if you're interested.

@emmett-b commented on GitHub (Apr 6, 2021): @zadjii-msft I can send you a debug mode minidump of the crash if you're interested.

claunia commented