Skip to content

Commit

Permalink
Fix a use-after-free crash when returning from the alt buffer (#10878)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

When switching from the alt buffer back to the main buffer, we need to copy certain cursor attributes from the one to the other. However, this copying was taking place after the alt buffer had been freed, and thus could result in the app crashing. This PR simply moves that code up a bit so it's prior to the buffer being freed.

## References

PR #10843 added the code that introduced this problem.

## PR Checklist
* [ ] Closes #xxx
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

## Validation Steps Performed

I was able to reproduce the crash when using a debug build, and confirmed that the crash no longer occurred after this PR was applied. I also checked that the cursor attributes were still being correctly copied back when returning from the alt buffer.
  • Loading branch information
j4james authored and DHowett committed Aug 25, 2021
1 parent a18bca8 commit f6a821f
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/host/screenInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2002,8 +2002,6 @@ void SCREEN_INFORMATION::UseMainScreenBuffer()

SCREEN_INFORMATION* psiAlt = psiMain->_psiAlternateBuffer;
psiMain->_psiAlternateBuffer = nullptr;
s_RemoveScreenBuffer(psiAlt); // this will also delete the alt buffer
// deleting the alt buffer will give the GetSet back to its main

// Copy the alt buffer's cursor style and visibility back to the main buffer.
const auto& altCursor = psiAlt->GetTextBuffer().GetCursor();
Expand All @@ -2012,6 +2010,9 @@ void SCREEN_INFORMATION::UseMainScreenBuffer()
mainCursor.SetIsVisible(altCursor.IsVisible());
mainCursor.SetBlinkingAllowed(altCursor.IsBlinkingAllowed());

s_RemoveScreenBuffer(psiAlt); // this will also delete the alt buffer
// deleting the alt buffer will give the GetSet back to its main

// Tell the VT MouseInput handler that we're in the main buffer now
gci.GetActiveInputBuffer()->GetTerminalInput().UseMainScreenBuffer();
}
Expand Down

0 comments on commit f6a821f

Please sign in to comment.