From b6d94e7b13eb9a14edea843a79c6d86b5b6d8803 Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Fri, 13 Nov 2020 05:22:28 +0300 Subject: [PATCH] Fix use after free when dropping zerowidth data Commit ec42b42ce601808070462111c0c28edb0e89babb added an optional pointer for each cell, thus some old code that was optimizing copying with 'ptr::copy' was duplicating "unique" pointers ('Box'), which was resulting in use after free, after attempting to free both of these pointers. By replacing these unsafe blocks with safe Rust, the issue itself is fixed and the potential for future memory problems is eliminated from this area of the code. --- alacritty_terminal/src/term/mod.rs | 32 +++++++++++------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/alacritty_terminal/src/term/mod.rs b/alacritty_terminal/src/term/mod.rs index af64cb5e..6084d8b0 100644 --- a/alacritty_terminal/src/term/mod.rs +++ b/alacritty_terminal/src/term/mod.rs @@ -1486,12 +1486,10 @@ impl Handler for Term { if self.mode.contains(TermMode::INSERT) && self.grid.cursor.point.col + width < num_cols { let line = self.grid.cursor.point.line; let col = self.grid.cursor.point.col; - let line = &mut self.grid[line]; + let row = &mut self.grid[line][..]; - let src = line[col..].as_ptr(); - let dst = line[(col + width)..].as_mut_ptr(); - unsafe { - ptr::copy(src, dst, (num_cols - col - width).0); + for col in (col.0..(num_cols - width).0).rev() { + row.swap(col + width, col); } } @@ -1574,18 +1572,15 @@ impl Handler for Term { let num_cells = (self.cols() - destination).0; let line = cursor.point.line; - let row = &mut self.grid[line]; + let row = &mut self.grid[line][..]; - unsafe { - let src = row[source..].as_ptr(); - let dst = row[destination..].as_mut_ptr(); - - ptr::copy(src, dst, num_cells); + for offset in (0..num_cells).rev() { + row.swap(destination.0 + offset, source.0 + offset); } // Cells were just moved out toward the end of the line; // fill in between source and dest with blanks. - for cell in &mut row[source..destination] { + for cell in &mut row[source.0..destination.0] { *cell = bg.into(); } } @@ -1841,21 +1836,18 @@ impl Handler for Term { let start = cursor.point.col; let end = min(start + count, cols - 1); - let n = (cols - end).0; + let num_cells = (cols - end).0; let line = cursor.point.line; - let row = &mut self.grid[line]; + let row = &mut self.grid[line][..]; - unsafe { - let src = row[end..].as_ptr(); - let dst = row[start..].as_mut_ptr(); - - ptr::copy(src, dst, n); + for offset in 0..num_cells { + row.swap(start.0 + offset, end.0 + offset); } // Clear last `count` cells in the row. If deleting 1 char, need to delete // 1 cell. - let end = cols - count; + let end = (cols - count).0; for cell in &mut row[end..] { *cell = bg.into(); }