starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-04-11 16:51:00 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Fix SetConsoleWindowInfo being able to crash ConPTY (#13212)

Browse Source 
MSFT-33471786 is one of the most common crashes we have right now.
Memory dumps suggest that `VtEngine::UpdateViewport` is called with a rectangle
like `(0, 46, 119, 29)` (left, top, right, bottom), which is a rectangle of
negative height. When the `_invalidMap` is resized the negative size gets
turned into a very large unsigned integer, which results in an OOM exception,
crashing OpenConsole.

`VtEngine::UpdateViewport` is called by `Renderer::_CheckViewportAndScroll`
which holds a (cached) old and a new viewport. The old viewport was
`(0, 46, 119, 75)` which is exceedingly similar to the invalid, new viewport.
It's bottom coordinate is also coincidentally larger by exactly 46 (top).

The viewport comes from the `SCREEN_INFORMATION` class whose `SetViewport`
function was highly suspicious as it has a branch which updates the bottom
to be the buffer height, but leaves the top unmodified.

`SCREEN_INFORMATION::SetViewport` is called by `SetConsoleWindowInfo` which
processes user-provided data. A repro of the crash can be constructed with:
```
SMALL_RECT rect{0, 46, 119, 75};
SetConsoleWindowInfo(GetStdHandle(STD_OUTPUT_HANDLE), TRUE, &rect);
```

Closes #13193
Closes MSFT-33471786

## Validation Steps Performed
Ensured the following code doesn't crash when run under Windows Terminal:
```
SMALL_RECT rect{0, 46, 119, 75};
SetConsoleWindowInfo(GetStdHandle(STD_OUTPUT_HANDLE), TRUE, &rect);
```
This commit is contained in:
Leonard Hecker
2022-06-01 19:47:22 +02:00
committed by GitHub
parent fb1491a4af
commit 7dbe741e1a
1 changed files with 10 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
src/host/screenInfo.cpp
View File

@@ -2268,30 +2268,19 @@ void SCREEN_INFORMATION::SetViewport(const Viewport& newViewport,
}
// do adjustments on a copy that's easily manipulated.
auto srCorrected = newViewport.ToExclusive();
const til::rect viewportRect{ newViewport.ToInclusive() };
const til::size coordScreenBufferSize{ GetBufferSize().Dimensions() };
if (srCorrected.Left < 0)
{
srCorrected.Right -= srCorrected.Left;
srCorrected.Left = 0;
}
if (srCorrected.Top < 0)
{
srCorrected.Bottom -= srCorrected.Top;
srCorrected.Top = 0;
}
// MSFT-33471786, GH#13193:
// newViewport may reside anywhere outside of the valid coordScreenBufferSize.
// For instance it might be scrolled down more than our text buffer allows to be scrolled.
const auto cx = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.width(), 1, coordScreenBufferSize.width));
const auto cy = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.height(), 1, coordScreenBufferSize.height));
const auto x = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.left, 0, coordScreenBufferSize.width - cx));
const auto y = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.top, 0, coordScreenBufferSize.height - cy));
const auto coordScreenBufferSize = GetBufferSize().Dimensions();
if (srCorrected.Right > coordScreenBufferSize.X)
{
srCorrected.Right = coordScreenBufferSize.X;
}
if (srCorrected.Bottom > coordScreenBufferSize.Y)
{
srCorrected.Bottom = coordScreenBufferSize.Y;
}
_viewport = Viewport::FromExclusive({ x, y, x + cx, y + cy });
_viewport = Viewport::FromExclusive(srCorrected);
if (updateBottom)
{
UpdateBottom();