Repository: microsoft/terminal (language: cpp)

Issue:
NULL_CLASS_PTR_READ from PublicTerminalCore.dll!TerminalIsSelectionActive
### Windows Terminal version

1.15.2210.6001

### Windows build number

10.0.19044.2251

### Other Software

Visual Studio 2022 v.17.5.33103.201

### Steps to reproduce

Currently unknown, but the dumps show the user selecting some text and Visual Studio crashing due to an AccessViolationException.

You can find more information in the Watson cabs: 
https://watsonportal.microsoft.com/Failure?FailureSearchText=32503620-0791-0795-1eff-dc016b5e23e9&MaxRows=100

Internal bug ticket:
[Bug 1676887](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1676887): [Watson] crash64: NULL_CLASS_PTR_READ_c0000005_PublicTerminalCore.dll!TerminalIsSelectionActive

Version of VS showing this issue:
https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/1639645
(Go to Build and Install tab for download link)

### Expected Behavior

_No response_

### Actual Behavior

Visual Studio crashes when text is selected


Discussion / hints:
I can't get at the dumps on this machine, but the blamed stack frame is in 

https://github.com/microsoft/terminal/blob/f2eed92345f86424d0c0e54839626a19643f0e9c/src/cascadia/PublicTerminalCore/HwndTerminal.cpp#L609-L614

There's no guards there. There probably should be, but also, in what scenario is there ever a null `publicTerminal`, or a null `publicTerminal->_terminal`? I'm pretty sure there shouldn't be...
![image](https://user-images.githubusercontent.com/18356694/205143305-c95ea8d7-2343-417c-b1bf-8bcd9bc8afa7.png)

```
0b 00000077`7a8fd4c0 00007ffb`8c358530     ntdll!KiUserExceptionDispatch+0x2e [minkernel\ntos\rtl\amd64\trampoln.asm @ 755] 
0c 00000077`7a8fdc68 00007ffb`a75ce837     PublicTerminalCore!TerminalIsSelectionActive [C:\a\_work\1\s\src\cascadia\PublicTerminalCore\HwndTerminal.cpp @ 610] 
0d 00000077`7a8fdc70 00007ffb`a75ce774     0x00007ffb`a75ce837
0e 00000077`7a8fdd20 00007ffb`a75ce677     0x00007ffb`a75ce774
0f 00000077`7a8fdd50 00007ffb`8a86e417     0x00007ffb`a75ce677
...
```

well, that's... not what I'd expect

Fix patch:
diff --git a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp
index 306c0695b54..7139f3949b8 100644
--- a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp
+++ b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp
@@ -102,11 +102,11 @@ try
             }
             break;
         case WM_RBUTTONDOWN:
-            if (terminal->_terminal->IsSelectionActive())
+            if (const auto& termCore{ terminal->_terminal }; termCore && termCore->IsSelectionActive())
             {
                 try
                 {
-                    const auto bufferData = terminal->_terminal->RetrieveSelectedTextFromBuffer(false);
+                    const auto bufferData = termCore->RetrieveSelectedTextFromBuffer(false);
                     LOG_IF_FAILED(terminal->_CopyTextToSystemClipboard(bufferData, true));
                     TerminalClearSelection(terminal);
                 }
@@ -260,6 +260,10 @@ CATCH_LOG();
 
 void HwndTerminal::RegisterScrollCallback(std::function<void(int, int, int)> callback)
 {
+    if (!_terminal)
+    {
+        return;
+    }
     _terminal->SetScrollPositionChangedCallback(callback);
 }
 
@@ -295,6 +299,10 @@ HWND HwndTerminal::GetHwnd() const noexcept
 
 void HwndTerminal::_UpdateFont(int newDpi)
 {
+    if (!_terminal)
+    {
+        return;
+    }
     _currentDpi = newDpi;
     auto lock = _terminal->LockForWriting();
 
@@ -311,6 +319,10 @@ IRawElementProviderSimple* HwndTerminal::_GetUiaProvider() noexcept
     {
         try
         {
+            if (!_terminal)
+            {
+                return nullptr;
+            }
             auto lock = _terminal->LockForWriting();
             LOG_IF_FAILED(::Microsoft::WRL::MakeAndInitialize<HwndTerminalAutomationPeer>(&_uiaProvider, this->GetRenderData(), this));
             _uiaEngine = std::make_unique<::Microsoft::Console::Render::UiaEngine>(_uiaProvider.Get());
@@ -329,6 +341,7 @@ IRawElementProviderSimple* HwndTerminal::_GetUiaProvider() noexcept
 
 HRESULT HwndTerminal::Refresh(const til::size windowSize, _Out_ til::size* dimensions)
 {
+    RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _terminal);
     RETURN_HR_IF_NULL(E_INVALIDARG, dimensions);
 
     auto lock = _terminal->LockForWriting();
@@ -364,6 +377,10 @@ HRESULT HwndTerminal::Refresh(const til::size windowSize, _Out_ til::size* dimen
 
 void HwndTerminal::SendOutput(std::wstring_view data)
 {
+    if (!_terminal)
+    {
+        return;
+    }
     _terminal->Write(data);
 }
 
@@ -474,8 +491,10 @@ void _stdcall TerminalDpiChanged(void* terminal, int newDpi)
 
 void _stdcall TerminalUserScroll(void* terminal, int viewTop)
 {
-    const auto publicTerminal = static_cast<const HwndTerminal*>(terminal);
-    publicTerminal->_terminal->UserScrollViewport(viewTop);
+    if (const auto publicTerminal = static_cast<const HwndTerminal*>(terminal); publicTerminal && publicTerminal->_terminal)
+    {
+        publicTerminal->_terminal->UserScrollViewport(viewTop);
+    }
 }
 
 const unsigned int HwndTerminal::_NumberOfClicks(til::point point, std::chrono::steady_clock::time_point timestamp) noexcept
@@ -497,6 +516,7 @@ const unsigned int HwndTerminal::_NumberOfClicks(til::point point, std::chrono::
 HRESULT HwndTerminal::_StartSelection(LPARAM lParam) noexcept
 try
 {
+    RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _terminal);
     const til::point cursorPosition{
         GET_X_LPARAM(lParam),
         GET_Y_LPARAM(lParam),
@@ -540,6 +560,7 @@ CATCH_RETURN();
 HRESULT HwndTerminal::_MoveSelection(LPARAM lParam) noexcept
 try
 {
+    RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _terminal);
     const til::point cursorPosition{
         GET_X_LPARAM(lParam),
         GET_Y_LPARAM(lParam),
@@ -578,6 +599,10 @@ CATCH_RETURN();
 void HwndTerminal::_ClearSelection() noexcept
 try
 {
+    if (!_terminal)
+    {
+        return;
+    }
     auto lock{ _terminal->LockForWriting() };
     _terminal->ClearSelection();
     _renderer->TriggerSelection();
@@ -592,15 +617,21 @@ void _stdcall TerminalClearSelection(void* terminal)
 
 bool _stdcall TerminalIsSelectionActive(void* terminal)
 {
-    const auto publicTerminal = static_cast<const HwndTerminal*>(terminal);
-    const auto selectionActive = publicTerminal->_terminal->IsSelectionActive();
-    return selectionActive;
+    if (const auto publicTerminal = static_cast<const HwndTerminal*>(terminal); publicTerminal && publicTerminal->_terminal)
+    {
+        return publicTerminal->_terminal->IsSelectionActive();
+    }
+    return false;
 }
 
 // Returns the selected text in the terminal.
 const wchar_t* _stdcall TerminalGetSelection(void* terminal)
 {
     auto publicTerminal = static_cast<HwndTerminal*>(terminal);
+    if (!publicTerminal || !publicTerminal->_terminal)
+    {
+        return nullptr;
+    }
 
     const auto bufferData = publicTerminal->_terminal->RetrieveSelectedTextFromBuffer(false);
     publicTerminal->_ClearSelection();
@@ -652,12 +683,17 @@ bool HwndTerminal::_CanSendVTMouseInput() const noexcept
 {
     // Only allow the transit of mouse events if shift isn't pressed.
     const auto shiftPressed = GetKeyState(VK_SHIFT) < 0;
-    return !shiftPressed && _focused && _terminal->IsTrackingMouseInput();
+    return !shiftPressed && _focused && _terminal && _terminal->IsTrackingMouseInput();
 }
 
 bool HwndTerminal::_SendMouseEvent(UINT uMsg, WPARAM wParam, LPARAM lParam) noexcept
 try
 {
+    if (!_terminal)
+    {
+        return false;
+    }
+
     til::point cursorPosition{
         GET_X_LPARAM(lParam),
         GET_Y_LPARAM(lParam),
@@ -690,6 +726,11 @@ catch (...)
 void HwndTerminal::_SendKeyEvent(WORD vkey, WORD scanCode, WORD flags, bool keyDown) noexcept
 try
 {
+    if (!_terminal)
+    {
+        return;
+    }
+
     auto modifiers = getControlKeyState();
     if (WI_IsFlagSet(flags, ENHANCED_KEY))
     {
@@ -706,7 +747,11 @@ CATCH_LOG();
 void HwndTerminal::_SendCharEvent(wchar_t ch, WORD scanCode, WORD flags) noexcept
 try
 {
-    if (_terminal->IsSelectionActive())
+    if (!_terminal)
+    {
+        return;
+    }
+    else if (_terminal->IsSelectionActive())
     {
         _ClearSelection();
         if (ch == UNICODE_ESC)
@@ -754,6 +799,10 @@ void _stdcall DestroyTerminal(void* terminal)
 void _stdcall TerminalSetTheme(void* terminal, TerminalTheme theme, LPCWSTR fontFamily, til::CoordType fontSize, int newDpi)
 {
     const auto publicTerminal = static_cast<HwndTerminal*>(terminal);
+    if (!publicTerminal || !publicTerminal->_terminal)
+    {
+        return;
+    }
     {
         auto lock = publicTerminal->_terminal->LockForWriting();
 
@@ -789,7 +838,7 @@ void _stdcall TerminalSetTheme(void* terminal, TerminalTheme theme, LPCWSTR font
 void _stdcall TerminalBlinkCursor(void* terminal)
 {
     const auto publicTerminal = static_cast<const HwndTerminal*>(terminal);
-    if (!publicTerminal->_terminal->IsCursorBlinkingAllowed() && publicTerminal->_terminal->IsCursorVisible())
+    if (!publicTerminal || !publicTerminal->_terminal || (!publicTerminal->_terminal->IsCursorBlinkingAllowed() && publicTerminal->_terminal->IsCursorVisible()))
     {
         return;
     }
@@ -800,6 +849,10 @@ void _stdcall TerminalBlinkCursor(void* terminal)
 void _stdcall TerminalSetCursorVisible(void* terminal, const bool visible)
 {
     const auto publicTerminal = static_cast<const HwndTerminal*>(terminal);
+    if (!publicTerminal || !publicTerminal->_terminal)
+    {
+        return;
+    }
     publicTerminal->_terminal->SetCursorOn(visible);
 }
 
@@ -831,6 +884,7 @@ void __stdcall TerminalKillFocus(void* terminal)
 HRESULT HwndTerminal::_CopyTextToSystemClipboard(const TextBuffer::TextAndColor& rows, const bool fAlsoCopyFormatting)
 try
 {
+    RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _terminal);
     std::wstring finalString;
 
     // Concatenate strings into one giant string to put onto the clipboard.
@@ -987,6 +1041,10 @@ double HwndTerminal::GetScaleFactor() const noexcept
 
 void HwndTerminal::ChangeViewport(const til::inclusive_rect& NewWindow)
 {
+    if (!_terminal)
+    {
+        return;
+    }
     _terminal->UserScrollViewport(NewWindow.top);
 }
 


Test patch: