From e3e57081a244c27e738b9a26dbb1cc418a4781fa Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 21 Apr 2020 09:45:17 -0700 Subject: [PATCH] UIA: Prevent crash from invalid UTR endpoint comparison (#5399) ## Summary of the Pull Request This is a quick-and-easy solution to #5309. If the ITextRangeProvider API allows us to take in two UiaTextRanges, we need to verify that they are both valid. With this PR, we make sure they both fit in the current TextBuffer. If not, we return `E_FAIL`. Though this doesn't prove that both UiaTextRanges are from the same TextBuffer, at the very least we don't crash and in cases where we can't make a valid comparison, we return an HRESULT failure. ## References #5406 - This should be the proper solution to this problem. Each UiaTextRange needs to be aware of which TextBuffer it came from. ## PR Checklist * [X] Closes #5309 ## Validation Steps Performed 1. generate enough output to cause the terminal to scroll 2. execute `nano` to make us go into the alternate buffer This previously crashed, now NVDA seems to detect that there was an error and keeps moving along. --- src/types/UiaTextRangeBase.cpp | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index d8354263ea4..3245620e369 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -227,8 +227,17 @@ try // get the values of our endpoint const auto mine = GetEndpoint(endpoint); + // TODO GH#5406: create a different UIA parent object for each TextBuffer + // This is a temporary solution to comparing two UTRs from different TextBuffers + // Ensure both endpoints fit in the current buffer. + const auto bufferSize = _pData->GetTextBuffer().GetSize(); + if (!bufferSize.IsInBounds(mine, true) || !bufferSize.IsInBounds(other, true)) + { + return E_FAIL; + } + // compare them - *pRetVal = _pData->GetTextBuffer().GetSize().CompareInBounds(mine, other, true); + *pRetVal = bufferSize.CompareInBounds(mine, other, true); UiaTracing::TextRange::CompareEndpoints(*this, endpoint, *range, targetEndpoint, *pRetVal); return S_OK; @@ -654,6 +663,17 @@ try return E_INVALIDARG; } + // TODO GH#5406: create a different UIA parent object for each TextBuffer + // This is a temporary solution to comparing two UTRs from different TextBuffers + // Ensure both endpoints fit in the current buffer. + const auto bufferSize = _pData->GetTextBuffer().GetSize(); + const auto mine = GetEndpoint(endpoint); + const auto other = range->GetEndpoint(targetEndpoint); + if (!bufferSize.IsInBounds(mine, true) || !bufferSize.IsInBounds(other, true)) + { + return E_FAIL; + } + SetEndpoint(endpoint, range->GetEndpoint(targetEndpoint)); UiaTracing::TextRange::MoveEndpointByRange(endpoint, *range, targetEndpoint, *this); @@ -772,9 +792,6 @@ CATCH_RETURN(); IFACEMETHODIMP UiaTextRangeBase::GetChildren(_Outptr_result_maybenull_ SAFEARRAY** ppRetVal) noexcept { - // TODO GitHub #1914: Re-attach Tracing to UIA Tree - //Tracing::s_TraceUia(this, ApiCall::GetChildren, nullptr); - RETURN_HR_IF(E_INVALIDARG, ppRetVal == nullptr); // we don't have any children