Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clamp VT parameter values to 32767 #5200

Merged
1 commit merged into from
Apr 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions src/terminal/parser/stateMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ void StateMachine::_ActionIgnore() noexcept
// - wch - Character to collect.
// Return Value:
// - <none>
void StateMachine::_ActionOscParam(const wchar_t wch)
void StateMachine::_ActionOscParam(const wchar_t wch) noexcept
{
_trace.TraceOnAction(L"OscParamCollect");

Expand Down Expand Up @@ -1002,7 +1002,7 @@ void StateMachine::_EventCsiParam(const wchar_t wch)
// - wch - Character that triggered the event
// Return Value:
// - <none>
void StateMachine::_EventOscParam(const wchar_t wch)
void StateMachine::_EventOscParam(const wchar_t wch) noexcept
{
_trace.TraceOnEvent(L"OscParam");
if (_isOscTerminator(wch))
Expand Down Expand Up @@ -1416,20 +1416,21 @@ void StateMachine::ResetState() noexcept
// into the given size_t. All existing value is moved up by 10.
// - For example, if your value had 437 and you put in the printable number 2,
// this function will update value to 4372.
// - Clamps to size_t max if it gets too big.
// - Clamps to 32767 if it gets too big.
// Arguments:
// - wch - Printable character to accumulate into the value (after conversion to number, of course)
// - value - The value to update with the printable character. See example above.
// Return Value:
// - <none> - But really it's the update to the given value parameter.
void StateMachine::_AccumulateTo(const wchar_t wch, size_t& value)
void StateMachine::_AccumulateTo(const wchar_t wch, size_t& value) noexcept
{
const size_t digit = wch - L'0';

// If we overflow while multiplying and adding, the value is just size_t max.
if (FAILED(SizeTMult(value, 10, &value)) ||
FAILED(SizeTAdd(value, digit, &value)))
value = value * 10 + digit;

// Values larger than the maximum should be mapped to the largest supported value.
if (value > MAX_PARAMETER_VALUE)
{
value = std::numeric_limits<size_t>().max();
value = MAX_PARAMETER_VALUE;
}
}
12 changes: 9 additions & 3 deletions src/terminal/parser/stateMachine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ Module Name:

namespace Microsoft::Console::VirtualTerminal
{
// The DEC STD 070 reference recommends supporting up to at least 16384 for
// parameter values, so 32767 should be more than enough. At most we might
// want to increase this to 65535, since that is what XTerm and VTE support,
// but for now 32767 is the safest limit for our existing code base.
constexpr size_t MAX_PARAMETER_VALUE = 32767;

class StateMachine final
{
#ifdef UNIT_TESTING
Expand Down Expand Up @@ -49,7 +55,7 @@ namespace Microsoft::Console::VirtualTerminal
void _ActionCollect(const wchar_t wch);
void _ActionParam(const wchar_t wch);
void _ActionCsiDispatch(const wchar_t wch);
void _ActionOscParam(const wchar_t wch);
void _ActionOscParam(const wchar_t wch) noexcept;
void _ActionOscPut(const wchar_t wch);
void _ActionOscDispatch(const wchar_t wch);
void _ActionSs3Dispatch(const wchar_t wch);
Expand Down Expand Up @@ -77,13 +83,13 @@ namespace Microsoft::Console::VirtualTerminal
void _EventCsiIntermediate(const wchar_t wch);
void _EventCsiIgnore(const wchar_t wch);
void _EventCsiParam(const wchar_t wch);
void _EventOscParam(const wchar_t wch);
void _EventOscParam(const wchar_t wch) noexcept;
void _EventOscString(const wchar_t wch);
void _EventOscTermination(const wchar_t wch);
void _EventSs3Entry(const wchar_t wch);
void _EventSs3Param(const wchar_t wch);

void _AccumulateTo(const wchar_t wch, size_t& value);
void _AccumulateTo(const wchar_t wch, size_t& value) noexcept;

enum class VTStates
{
Expand Down
13 changes: 9 additions & 4 deletions src/terminal/parser/ut_parser/OutputEngineTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final
mach.ProcessCharacter(wch);
VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::OscParam);
}
VERIFY_ARE_EQUAL(mach._oscParameter, sizeMax);
VERIFY_ARE_EQUAL(mach._oscParameter, MAX_PARAMETER_VALUE);
mach.ProcessCharacter(L';');
VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::OscString);
mach.ProcessCharacter(L's');
Expand All @@ -542,7 +542,7 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final
mach.ProcessCharacter(wch);
VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::OscParam);
}
VERIFY_ARE_EQUAL(mach._oscParameter, sizeMax);
VERIFY_ARE_EQUAL(mach._oscParameter, MAX_PARAMETER_VALUE);
mach.ProcessCharacter(L';');
VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::OscString);
mach.ProcessCharacter(L's');
Expand Down Expand Up @@ -1064,15 +1064,20 @@ class StateMachineExternalTest final
void ApplyParameterBoundary(size_t* uiExpected, size_t uiGiven)
{
// 0 and 1 should be 1. Use the preset value.
// 2019-12: No longer bound by SHORT_MAX. Goes all the way to size_t.
// 1-MAX_PARAMETER_VALUE should be what we set.
// > MAX_PARAMETER_VALUE should be MAX_PARAMETER_VALUE.
if (uiGiven <= 1)
{
*uiExpected = 1u;
}
else if (uiGiven > 1)
else if (uiGiven > 1 && uiGiven <= MAX_PARAMETER_VALUE)
{
*uiExpected = uiGiven;
}
else if (uiGiven > MAX_PARAMETER_VALUE)
{
*uiExpected = MAX_PARAMETER_VALUE; // 32767 is our max value.
}
}

void TestCsiCursorMovement(wchar_t const wchCommand,
Expand Down