Repository: microsoft/terminal (language: cpp)

Issue:
Certain sequences with default parameters can trigger a crash
### Windows Terminal version

1.17.1023

### Windows build number

10.0.19045.2728

### Other Software

_No response_

### Steps to reproduce

1. Open a bash shell.
2. Execute `printf "\e[\$r"`


### Expected Behavior

That sequence executes the `DECCARA` control with all default parameters. That should have the effect of resetting the attributes of everything on the screen.

### Actual Behavior

The terminal hangs for a second, and then the tab closes (assumedly because the underlying conhost has crashed).

The problem arises when using the `VTParameters::subspan` method to access an offset beyond the end of the original span. Other than the `DECCARA` sequence, this can also be triggered by `DECRARA` and `DECPS`. In the latter case, the whole terminal can crash rather than just the active tab. That may just be a matter of chance though.

My suggested fix would be to change the `VTParameters::subspan` implementation here:

https://github.com/microsoft/terminal/blob/5ed3c76dcbf2d1ad7dc57c599c85c85713b0294f/src/terminal/adapter/DispatchTypes.hpp#L187

to something like this:

````cpp
    const auto subValues = _values.subspan(std::min(offset, _values.size()));
````

That way an offset beyond the end of the source span would just produce an empty span (that's what I mistakenly assumed the `subspan` was already doing).


Discussion / hints:

Fix patch:
diff --git a/src/terminal/adapter/DispatchTypes.hpp b/src/terminal/adapter/DispatchTypes.hpp
index 0f8b6ede82c..3b3d987665d 100644
--- a/src/terminal/adapter/DispatchTypes.hpp
+++ b/src/terminal/adapter/DispatchTypes.hpp
@@ -184,7 +184,7 @@ namespace Microsoft::Console::VirtualTerminal
 
         VTParameters subspan(const size_t offset) const noexcept
         {
-            const auto subValues = _values.subspan(offset);
+            const auto subValues = _values.subspan(std::min(offset, _values.size()));
             return { subValues.data(), subValues.size() };
         }
 
diff --git a/src/terminal/parser/ut_parser/StateMachineTest.cpp b/src/terminal/parser/ut_parser/StateMachineTest.cpp
index 125036c16f7..4893964c249 100644
--- a/src/terminal/parser/ut_parser/StateMachineTest.cpp
+++ b/src/terminal/parser/ut_parser/StateMachineTest.cpp
@@ -157,6 +157,8 @@ class Microsoft::Console::VirtualTerminal::StateMachineTest
     TEST_METHOD(PassThroughUnhandledSplitAcrossWrites);
 
     TEST_METHOD(DcsDataStringsReceivedByHandler);
+
+    TEST_METHOD(VtParameterSubspanTest);
 };
 
 void StateMachineTest::TwoStateMachinesDoNotInterfereWithEachOther()
@@ -337,3 +339,38 @@ void StateMachineTest::DcsDataStringsReceivedByHandler()
     // Verify the control characters were executed (if expected).
     VERIFY_ARE_EQUAL(expectedExecuted, engine.executed);
 }
+
+void StateMachineTest::VtParameterSubspanTest()
+{
+    const auto parameterList = std::vector<VTParameter>{ 12, 34, 56, 78 };
+    const auto parameterSpan = VTParameters{ parameterList.data(), parameterList.size() };
+
+    {
+        Log::Comment(L"Subspan from 0 gives all the parameters");
+        const auto subspan = parameterSpan.subspan(0);
+        VERIFY_ARE_EQUAL(4u, subspan.size());
+        VERIFY_ARE_EQUAL(12, subspan.at(0));
+        VERIFY_ARE_EQUAL(34, subspan.at(1));
+        VERIFY_ARE_EQUAL(56, subspan.at(2));
+        VERIFY_ARE_EQUAL(78, subspan.at(3));
+    }
+    {
+        Log::Comment(L"Subspan from 2 gives the last 2 parameters");
+        const auto subspan = parameterSpan.subspan(2);
+        VERIFY_ARE_EQUAL(2u, subspan.size());
+        VERIFY_ARE_EQUAL(56, subspan.at(0));
+        VERIFY_ARE_EQUAL(78, subspan.at(1));
+    }
+    {
+        Log::Comment(L"Subspan at the end of the range gives 1 omitted value");
+        const auto subspan = parameterSpan.subspan(4);
+        VERIFY_ARE_EQUAL(1u, subspan.size());
+        VERIFY_IS_FALSE(subspan.at(0).has_value());
+    }
+    {
+        Log::Comment(L"Subspan past the end of the range gives 1 omitted value");
+        const auto subspan = parameterSpan.subspan(6);
+        VERIFY_ARE_EQUAL(1u, subspan.size());
+        VERIFY_IS_FALSE(subspan.at(0).has_value());
+    }
+}


Test patch: