Summary
cursor::sys::windows::move_up and move_left subtract the caller's count from the current
cursor row/column with a bare u16 subtraction and no floor. If count exceeds the current row
or column the subtraction underflows: panic in a debug build, wrap to ~65535 in a release
build.
This is the legacy-console path only (reached through execute_winapi, i.e. when supports_ansi
is false), so it is Windows-only and does not affect the ANSI path.
The sites
src/cursor/sys/windows.rs on the default branch:
pub(crate) fn move_up(count: u16) -> std::io::Result<()> {
let (column, row) = position()?;
move_to(column, row - count)?; // :59 -- underflows when count > row
Ok(())
}
pub(crate) fn move_left(count: u16) -> std::io::Result<()> {
let (column, row) = position()?;
move_to(column - count, row)?; // :77 -- underflows when count > column
Ok(())
}MoveUp and MoveLeft in src/cursor.rs reach these through execute_winapi with the caller's
count unchecked, so queue!(w, MoveUp(5)) with the cursor on row 2 is sufficient to trigger it.
In release the wrapped value then feeds move_to, which casts to i16, sees a negative, and
returns an Argument Out of Range error — so the release outcome degrades to an error rather than
a bogus cursor move. That is better than a silent wrong position, but it is still not the intended
behaviour, and debug builds panic outright.
Why this reads as an oversight rather than a decision
The analogous case is guarded two impls away in the same file. MoveToNextLine and
MoveToPreviousLine both test their count before calling into the sys layer, while MoveUp,
MoveDown, MoveLeft and MoveRight call theirs unguarded. The author was thinking about the
edge in one place and not the neighbouring one.
Severity, honestly bounded
Not memory-unsafe: u16 subtraction panics or wraps, it does not read out of bounds. The trigger
is a caller passing a count larger than the current position, which is an ordinary thing for
application code to do near the top or left edge of the screen. No untrusted input is involved.
Suggested fix
saturating_sub matches what the crate already chose for the coordinate parsers in
src/event/sys/unix/parse.rs, and clamping at the edge is the natural terminal behaviour:
move_to(column, row.saturating_sub(count))?; // move_up
move_to(column.saturating_sub(count), row)?; // move_leftmove_down and move_right add rather than subtract, so they are not affected by this, though a
matching saturating_add would be consistent.
Scope note — the parser half of this report is already fixed
This finding originally covered eight bare - 1 subtractions in four coordinate parsers in
src/event/sys/unix/parse.rs. Checked against the default branch before filing: those are all
fixed — the parsers now use saturating_sub(1), parse_csi_normal_mouse folds the protocol
offset into a single saturating_sub(33), and there is a regression test
(test_parse_csi_normal_mouse_zero_coords_clamp) naming the old behaviour. Nothing is being
re-reported there; this issue is narrowed to the two Windows sites, which are unchanged.
Provenance
Found by a line-by-line read of crossterm 0.28.1 during a cargo vet supply-chain certification,
then re-checked against the default branch immediately before filing. Not observed on a live legacy
Windows console — the underflow is reasoned from the source, and the release-path behaviour is
derived from move_to's i16 cast rather than measured.
Activity
0No activity yet.