Ensure cursor is clamped after buffer swap #124
Conversation
vt_resize resizes both buffers of the given Vt* (involving a realloc), but can only correctly clamp the cursor of the active buffer. This means that when it comes time to switch to the other buffer in interpret_csi_priv_mode, we might be switching to a buffer which has a cursor pointing to old memory. Thus, when we switch buffers it's necessary to ensure the cursor is clamped to avoid memory errors. This is a bug I've observed for a few years but never often enough to worry me. After I was able to pin it down to activities such as opening of manpages and resizing terminals, I boiled it down to be reproducible as: 1. Open a manpage in dvtm, buffer swap to alt 2. Close the manpage and return to the shell, buffer swaps to norm 3. Resize the pane to have fewer rows than before, alt+norm are resized but only norm has its cursor clamped 4. Open a manpage again, UAF causes crash since unclamped curs_row on alt buffer is still pointing to before-resize allocation With some exploratory testing I have seen crashes identical and nearly identical to the following fixed by this patch: * martanne#73 * martanne#74
|
Marking this as draft until I've had some more time using this patch as daily driver to ensure there aren't regressions, but I'd appreciate if review/approval could be given in the meantime 😀 |
|
This is likely the bug that has been bothering me for a while. Occasionally dvtm crashes when using I'll be running this patch as well. Thank you for finding it! |
|
Marking this as ready. I've run this as my daily at home and at work and haven't seen any issues. @martanne if you get a chance to look at this that'd be great 👍 |
|
Looks like a great bug fix, I will also test it out! |
|
@martanne I forgot about this MR - have you had a chance to take a look at it by any chance? |
|
Works great so far. Thanks for investigating this issue! It's been annoying me for a while, glad someone's already done research on it. |
vt_resize resizes both buffers of the given Vt* (involving a realloc),
but can only correctly clamp the cursor of the active buffer. This means
that when it comes time to switch to the other buffer in
interpret_csi_priv_mode, we might be switching to a buffer which has a
cursor pointing to old memory. Thus, when we switch buffers it's
necessary to ensure the cursor is clamped to avoid memory errors.
This is a bug I've observed for a few years but never often enough to
worry me. After I was able to pin it down to activities such as opening
of manpages and resizing terminals, I boiled it down to be reproducible
as:
but only norm has its cursor clamped
alt buffer is still pointing to before-resize allocation
With some exploratory testing I have seen crashes identical and nearly
identical to the following fixed by this patch: