From 9ba20805ec4da6494a7e2c85666e007441419cb7 Mon Sep 17 00:00:00 2001
From: James Holderness <j4_james@hotmail.com>
Date: Mon, 2 Aug 2021 22:04:17 +0100
Subject: [PATCH] Sanitize C1 control chars in SetConsoleTitle API (#10847)

When the `SetContoleTitle` API is called with a title containing control
characters, we need to filter out those characters before we can forward
the title change over conpty as an escape sequence. If we don't do that,
the receiving terminal will end up executing the control characters
instead of updating the title. We were already filtering out the C0
control characters, but with this PR we're now filtering out C1 controls
characters as well.

I've simply updated the sanitizing routine in `DoSrvSetConsoleTitleW` to
filter our characters in the range `0x80` to `0x9F`. This is in addition
to the C0 range (`0x00` to `0x1F`) that was already excluded.

## Validation Steps Performed

I've added a conpty unit test that calls `DoSrvSetConsoleTitleW` with
titles containing a variety of C0 and C1 controls characters, and which
verifies that those characters are stripped from the title forwarded to
conpty.

I've also confirmed that the test case in issue #10312 is now working
correctly in Windows Terminal.

Closes #10312
---
 src/host/getset.cpp                    | 14 ++++-------
 src/host/ut_host/ConptyOutputTests.cpp | 35 ++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/src/host/getset.cpp b/src/host/getset.cpp
index dc0459f6d..6e9e70191 100644
--- a/src/host/getset.cpp
+++ b/src/host/getset.cpp
@@ -1931,15 +1931,11 @@ void DoSrvPrivateRefreshWindow(_In_ const SCREEN_INFORMATION& screenInfo)
     //      to embed control characters in that string.
     if (gci.IsInVtIoMode())
     {
-        std::wstring sanitized;
-        sanitized.reserve(title.size());
-        for (size_t i = 0; i < title.size(); i++)
-        {
-            if (title.at(i) >= UNICODE_SPACE)
-            {
-                sanitized.push_back(title.at(i));
-            }
-        }
+        std::wstring sanitized{ title };
+        sanitized.erase(std::remove_if(sanitized.begin(), sanitized.end(), [](auto ch) {
+                            return ch < UNICODE_SPACE || (ch > UNICODE_DEL && ch < UNICODE_NBSP);
+                        }),
+                        sanitized.end());
 
         gci.SetTitle({ sanitized });
     }
diff --git a/src/host/ut_host/ConptyOutputTests.cpp b/src/host/ut_host/ConptyOutputTests.cpp
index a76d5c4f8..d412fb562 100644
--- a/src/host/ut_host/ConptyOutputTests.cpp
+++ b/src/host/ut_host/ConptyOutputTests.cpp
@@ -119,6 +119,7 @@ class ConptyOutputTests
     TEST_METHOD(WriteTwoLinesUsesNewline);
     TEST_METHOD(WriteAFewSimpleLines);
     TEST_METHOD(InvalidateUntilOneBeforeEnd);
+    TEST_METHOD(SetConsoleTitleWithControlChars);
 
 private:
     bool _writeCallback(const char* const pch, size_t const cch);
@@ -364,3 +365,37 @@ void ConptyOutputTests::InvalidateUntilOneBeforeEnd()
 
     VERIFY_SUCCEEDED(renderer.PaintFrame());
 }
+
+void ConptyOutputTests::SetConsoleTitleWithControlChars()
+{
+    BEGIN_TEST_METHOD_PROPERTIES()
+        TEST_METHOD_PROPERTY(L"Data:control", L"{0x00, 0x0A, 0x1B, 0x80, 0x9B, 0x9C}")
+    END_TEST_METHOD_PROPERTIES()
+
+    int control;
+    VERIFY_SUCCEEDED(TestData::TryGetValue(L"control", control));
+
+    auto& g = ServiceLocator::LocateGlobals();
+    auto& renderer = *g.pRender;
+
+    Log::Comment(NoThrowString().Format(
+        L"SetConsoleTitle with a control character (0x%02X) embedded in the text", control));
+
+    std::wstringstream titleText;
+    titleText << L"Hello " << wchar_t(control) << L"World!";
+    VERIFY_SUCCEEDED(DoSrvSetConsoleTitleW(titleText.str()));
+
+    // This is the standard init sequences for the first frame.
+    expectedOutput.push_back("\x1b[2J");
+    expectedOutput.push_back("\x1b[m");
+    expectedOutput.push_back("\x1b[H");
+
+    // The title change is propagated as an OSC 0 sequence.
+    // Control characters are stripped, so it's always "Hello World".
+    expectedOutput.push_back("\x1b]0;Hello World!\a");
+
+    // This is also part of the standard init sequence.
+    expectedOutput.push_back("\x1b[?25h");
+
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+}