AtlasEngine: Fix a heap overflow bug #14275
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lhecker
added
Issue-Bug
lhecker
commented
Oct 21, 2022
| { | ||
| _api.invalidatedRows.x = 0; | ||
| _api.invalidatedRows.y = gsl::narrow_cast<u16>(clamp<int>(_api.invalidatedRows.y + delta, u16min, u16max)); | ||
| } | ||
| } |
There was a problem hiding this comment.
This change is way less egregious if you suppress whitespace changes.
This block of changes is unrelated to this PR but represent an earlier attempt at fixing it. I left it in because it'll help with implementing horizontal scrolling in the future.
| gsl::narrow_cast<u16>(srNewViewport.Right - srNewViewport.Left + 1), | ||
| gsl::narrow_cast<u16>(srNewViewport.Bottom - srNewViewport.Top + 1), | ||
| gsl::narrow_cast<u16>(std::max(1, srNewViewport.Right - srNewViewport.Left + 1)), | ||
| gsl::narrow_cast<u16>(std::max(1, srNewViewport.Bottom - srNewViewport.Top + 1)), |
There was a problem hiding this comment.
Prevents an orderly/expected crash when you resize the conhost window down to 0px height.
| std::fill_n(_api.bufferLineMetadata.data() + x, column - x, metadata); | ||
| } | ||
|
|
||
| _api.lastPaintBufferLineCoord = { x, y }; | ||
| _api.bufferLineWasHyperlinked = false; |
There was a problem hiding this comment.
This block moved down since I'm computing x late now.
| } | ||
|
|
||
| auto column = x; | ||
| clusters = clusters.subspan(offset); | ||
| } |
carlos-zamora
added a commit
that referenced
this pull request
Oct 25, 2022
zadjii-msft
approved these changes
Oct 31, 2022
DHowett
reviewed
Oct 31, 2022
| // into the first column. Since other code then blindly assumes that there's a | ||
| // preceding `Leading` character, we'll get called with a X coordinate of -1. | ||
| // | ||
| // This block can be removed after GH#13626 is merged. |
There was a problem hiding this comment.
Eh, we don't really have to. This is a small, self-contained snippet and so I feel like it can be removed whenever. But I'll add a note to make this change in my ROW PR.
DHowett
approved these changes
Oct 31, 2022
DHowett
added
the
AutoMerge
DHowett
pushed a commit
that referenced
this pull request
Dec 1, 2022
`TextBuffer` is buggy and allows a `Trailing` `DbcsAttribute` to be written into the first column. Since other code then blindly assumes that there's a preceding `Leading` character, we'll get called with a X coordinate of -1. This issue will be fixed by #13626 and this commit fixes it in the meantime. Additionally fixes an unimportant crash when the window height is 0px, because it was annoying during testing and doesn't hurt to be fixed. ## Validation Steps Performed * Run a stress test that prints random Unicode at random positions * Resize the window furiously at the same time * Doesn't crash / fail-fast ✅ (cherry picked from commit bb4711d) Service-Card-Id: 86353783 Service-Version: 1.16
|
🎉 Handy links: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TextBufferis buggy and allows aTrailingDbcsAttributeto be writteninto the first column. Since other code then blindly assumes that there's a
preceding
Leadingcharacter, we'll get called with a X coordinate of -1.This issue will be fixed by #13626 and this commit fixes it in the meantime.
Additionally fixes an unimportant crash when the window height is 0px,
because it was annoying during testing and doesn't hurt to be fixed.
Validation Steps Performed