Skip to content

Fix reproducible overflows in visual mode and boundary scans.#1

Open
Cherrling wants to merge 2 commits into
ziyao233:mainfrom
Cherrling:fix/patch-1
Open

Fix reproducible overflows in visual mode and boundary scans.#1
Cherrling wants to merge 2 commits into
ziyao233:mainfrom
Cherrling:fix/patch-1

Conversation

@Cherrling
Copy link
Copy Markdown

This change addresses four sanitizer-reproducible memory-safety issues in mvim.c with minimal code changes.

  • renderTrailingSpace() could walk backward past the start of a line when the line consisted entirely of spaces or tabs, causing an out-of-bounds read during screen refresh. The loop now checks the lower bound and uses iswspace() for wchar_t input.
  • The normal-mode dw path scanned forward without checking x against the row length. When deleting a word that reached end-of-line, the scan could step past the terminating null and continue reading out of bounds. The scan now stops at row->size.
  • The visual delete/cut path had two state bugs: it could compute an invalid history range for editorCommitChange(), and deleting the final remaining row could leave the editor with zero rows before later refresh/update logic ran. The change range calculation is corrected, the buffer is forced to retain at least one empty row after visual deletion, and the final row update is now guarded by y < E.numrows.
  • The visual-mode G path used custom cursor/refresh logic that could drive the screen state into an invalid range and trigger an out-of-bounds access while redrawing. It now reuses editorMoveCursorTo(E.numrows - 1, 0) and updates the visual range using the resulting cursor position.

These fixes were validated by rebuilding with AddressSanitizer and UndefinedBehaviorSanitizer and rerunning the previously crashing repro cases for trailing-space rendering, visual single-line delete, visual G, and dw at end-of-line.

Cherrling added 2 commits May 15, 2026 15:57
This change addresses four sanitizer-reproducible memory-safety issues in
mvim.c with minimal code changes.

- renderTrailingSpace() could walk backward past the start of a line when the
  line consisted entirely of spaces or tabs, causing an out-of-bounds read
  during screen refresh. The loop now checks the lower bound and uses
  iswspace() for wchar_t input.
- The normal-mode dw path scanned forward without checking x against the row
  length. When deleting a word that reached end-of-line, the scan could step
  past the terminating null and continue reading out of bounds. The scan now
  stops at row->size.
- The visual delete/cut path had two state bugs: it could compute an invalid
  history range for editorCommitChange(), and deleting the final remaining row
  could leave the editor with zero rows before later refresh/update logic ran.
  The change range calculation is corrected, the buffer is forced to retain at
  least one empty row after visual deletion, and the final row update is now
  guarded by y < E.numrows.
- The visual-mode G path used custom cursor/refresh logic that could drive the
  screen state into an invalid range and trigger an out-of-bounds access while
  redrawing. It now reuses editorMoveCursorTo(E.numrows - 1, 0) and updates the
  visual range using the resulting cursor position.

These fixes were validated by rebuilding with AddressSanitizer and
UndefinedBehaviorSanitizer and rerunning the previously crashing repro cases
for trailing-space rendering, visual single-line delete, visual G, and dw at
end-of-line.

Signed-off-by: Cherrling <me@cherr.cc>
After the earlier zero-row safeguard, full-line visual deletion still
returned early in the single-line path and skipped that guard. Repro:
open a one-line file, run v$dK or v$xK, then getKeywordUnderCursor()
reads freed row memory.

Signed-off-by: Cherrling <me@cherr.cc>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant