Skip to content

Commit

Permalink
UIA: Prevent crash from invalid UTR endpoint comparison (#5399)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
carlos-zamora authored Apr 21, 2020
1 parent e271599 commit e3e5708
Showing 1 changed file with 21 additions and 4 deletions.
25 changes: 21 additions & 4 deletions src/types/UiaTextRangeBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit e3e5708

Please sign in to comment.