Repository: microsoft/terminal (language: cpp)

Issue:
Crash in ScrollConsoleScreenBuffer when destination coordinates overflow
# Environment

Windows build number: Version 10.0.18362.657

# Steps to reproduce

Compile and run the following C++ code in a conhost cmd shell:
```cpp
#include <windows.h>

void main() {
    SMALL_RECT rect = {0, 0, 40, 0};
    COORD dest = {32767, 0};
    CHAR_INFO fill = {L'X', 0x07};
    HANDLE output = GetStdHandle(STD_OUTPUT_HANDLE);
    ScrollConsoleScreenBufferW(output, &rect, NULL, dest, &fill);
}
```

# Expected behavior

The first 40 characters of the first line should fill with X's.

# Actual behavior

The console crashes. 

When the `ScrollRegion` function is copying the area that is being scrolled, the `_CopyRectangle` function makes use of the `Viewport::WalkInBoundsCircular` method, and that fails when the target position goes out of bounds. See here:

https://github.com/microsoft/terminal/blob/5de9fa9cf375e9b5bdc582d8d166604eeb519844/src/types/viewport.cpp#L411





Discussion / hints:
FYI, I've just tested with the first open source release (commit d4d59fa3395012ca37ba12665da4ec11c7dcf9cb), and the bug was already present there. However, the legacy console doesn't have this issue.

In some ways this is could be considered a part of issue #4153, but I thought it worth raising separately because of the crash, and the fact that it's not obviously "safe math" code.
Regressed between RS5 (17763) and 19H1 (18362). Thanks!
Off triage, P2, /cc @miniksa for a conhost regression from the scrolling change in 19H1

Fix patch:
diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp
index f175ce010f7..e58b71fa3f0 100644
--- a/src/renderer/base/renderer.cpp
+++ b/src/renderer/base/renderer.cpp
@@ -677,8 +677,7 @@ void Renderer::_PaintBufferOutput(_In_ IRenderEngine* const pEngine)
 
     for (const auto& dirtyRect : dirtyAreas)
     {
-        // Shortcut: don't bother redrawing if the width is 0.
-        if (dirtyRect.left == dirtyRect.right)
+        if (!dirtyRect)
         {
             continue;
         }
diff --git a/src/types/viewport.cpp b/src/types/viewport.cpp
index 9038a60cca5..aa30501501f 100644
--- a/src/types/viewport.cpp
+++ b/src/types/viewport.cpp
@@ -46,7 +46,12 @@ Viewport Viewport::FromDimensions(const COORD origin,
                                   const short width,
                                   const short height) noexcept
 {
-    return Viewport::FromExclusive({ origin.X, origin.Y, origin.X + width, origin.Y + height });
+    return Viewport::FromInclusive({
+        origin.X,
+        origin.Y,
+        base::saturated_cast<short>(origin.X + width - 1),
+        base::saturated_cast<short>(origin.Y + height - 1),
+    });
 }
 
 // Function Description:
@@ -60,7 +65,12 @@ Viewport Viewport::FromDimensions(const COORD origin,
 Viewport Viewport::FromDimensions(const COORD origin,
                                   const COORD dimensions) noexcept
 {
-    return Viewport::FromExclusive({ origin.X, origin.Y, origin.X + dimensions.X, origin.Y + dimensions.Y });
+    return Viewport::FromInclusive({
+        origin.X,
+        origin.Y,
+        base::saturated_cast<short>(origin.X + dimensions.X - 1),
+        base::saturated_cast<short>(origin.Y + dimensions.Y - 1),
+    });
 }
 
 // Function Description:


Test patch: