Skip to content

Commit

Permalink
Prevent crash when VTParameters::subspan is out of range (#15235)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

There are certain escape sequences that use the `VTParameters::subspan`
method to access a subsection of the provided parameter list. When the
parameter list is empty, that `subspan` call can end up using an offset
that is out of range, which causes the terminal to crash. This PR stops
that from happening by clamping the offset so it's in range.

## References and Relevant Issues

This bug effected the `DECCARA` and `DECRARA` operations introduced in
PR #14285, and the `DECPS` operation introduced in PR #13208.

## Validation Steps Performed

I've manually confirmed that the sequences mentioned above are no longer
crashing when executed with an empty parameter list, and I've added a
little unit test that checks `VTParameters::subspan` method is returning
the expected results when passed an offset that is out of range.

## PR Checklist
- [x] Closes #15234
- [x] Tests added/passed
- [ ] Documentation updated
- [ ] Schema updated (if necessary)

(cherry picked from commit e413a41)
Service-Card-Id: 89001985
Service-Version: 1.17
  • Loading branch information
j4james authored and DHowett committed Apr 25, 2023
1 parent 5ca7991 commit 95636b9
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/terminal/adapter/DispatchTypes.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,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() };
}

Expand Down
37 changes: 37 additions & 0 deletions src/terminal/parser/ut_parser/StateMachineTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ class Microsoft::Console::VirtualTerminal::StateMachineTest
TEST_METHOD(PassThroughUnhandledSplitAcrossWrites);

TEST_METHOD(DcsDataStringsReceivedByHandler);

TEST_METHOD(VtParameterSubspanTest);
};

void StateMachineTest::TwoStateMachinesDoNotInterfereWithEachOther()
Expand Down Expand Up @@ -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());
}
}

0 comments on commit 95636b9

Please sign in to comment.