mirrors/neovim
1
0
Fork 0
mirror of https://github.com/neovim/neovim.git synced 2026-08-26 00:51:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(undo): crash on corrupted undo file #41339

Browse Source 
Problem:
`:rundo` on a corrupted undo file crashes or hangs, instead of failing
with E825. Patching one 4-byte field is enough:

    ue_size = 0xFFFFFFFF  " walks a NULL ue_array
    ue_size = 0x7FFFFFF0  " 17 GB xmalloc + memset, then preserve_exit()
    ue_top  = 0xFFFFFFFB  " negative lnum reaches ml_delete()

Analysis:
Every count in the file is read with `undo_read_4c()` and then checked,
differently at each site. None bounds the value by what the file can
hold, so a 2 GB count reaches `xmalloc()`.

Note:
- Vim doesn't have `bi_fsize` because it checks `U_ALLOC_LINE` result
  everywhere (thus doesn't crash, but may thrash...); those checks were
  dropped when Nvim moved to `xmalloc()`, and the `ue_size` loop counter
  became unsigned.
- Vim *does* have the negative line numbers bug: `u_undoredo()` checks
  `top > ml_line_count || top >= bot || bot > ml_line_count + 1`, which
  rejects none of them.

Solution:
- Introduce `undo_read_len()` and use it to fail early instead of
  continuing with nonsense.
- Validate `ue_top`/`ue_bot`/ `ue_lcount`.
- Use `xcalloc()`, so no site can proceed with a NULL array.
- Report a truncated "U" line, distinguish EOF from a 0xFFFFFFFF field,
  and free the header on the extmark error path.
This commit is contained in:
Justin M. Keyes
2026-08-16 10:24:24 -04:00
committed by GitHub
parent 83730db647
commit 214bcf24cc
2 changed files with 116 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

73
src/nvim/undo.c
View File

@@ -140,6 +140,7 @@
typedef struct {
buf_T *bi_buf;
FILE *bi_fp;
off_T bi_fsize; ///< Size of `bi_fp` when reading, 0 if unknown.
} bufinfo_T;
#include "undo.c.generated.h"
@@ -959,7 +960,7 @@ static u_header_T *unserialize_uhp(bufinfo_T *bi, const char *file_name)
last_uep->ue_next = uep;
}
last_uep = uep;
if (uep == NULL || error) {
if (error) {
u_free_uhp(uhp);
return NULL;
}
@@ -977,8 +978,8 @@ static u_header_T *unserialize_uhp(bufinfo_T *bi, const char *file_name)
bool error = false;
ExtmarkUndoObject *extup = unserialize_extmark(bi, &error, file_name);
if (error) {
kv_destroy(uhp->uh_extmark);
xfree(extup);
u_free_uhp(uhp);
return NULL;
}
kv_push(uhp->uh_extmark, *extup);
@@ -1079,6 +1080,7 @@ static bool serialize_uep(bufinfo_T *bi, u_entry_T *uep)
}
static u_entry_T *unserialize_uep(bufinfo_T *bi, bool *error, const char *file_name)
FUNC_ATTR_NONNULL_ALL FUNC_ATTR_NONNULL_RET
{
u_entry_T *uep = xmalloc(sizeof(u_entry_T));
CLEAR_POINTER(uep);
@@ -1088,26 +1090,26 @@ static u_entry_T *unserialize_uep(bufinfo_T *bi, bool *error, const char *file_n
uep->ue_top = undo_read_4c(bi);
uep->ue_bot = undo_read_4c(bi);
uep->ue_lcount = undo_read_4c(bi);
uep->ue_size = undo_read_4c(bi);
char **array = NULL;
if (uep->ue_size > 0) {
if ((size_t)uep->ue_size < SIZE_MAX / sizeof(char *)) {
array = xmalloc(sizeof(char *) * (size_t)uep->ue_size);
memset(array, 0, sizeof(char *) * (size_t)uep->ue_size);
}
if (uep->ue_top < 0 || uep->ue_bot < 0 || uep->ue_lcount < 0) {
// Fail early; u_undoredo() takes these as line numbers.
corruption_error("entry lnum", file_name);
*error = true;
return uep;
}
uep->ue_size = undo_read_len(bi, "entry size", file_name);
if (uep->ue_size < 0) {
uep->ue_size = 0; // u_freeentry() must not walk ue_array.
*error = true;
return uep;
}
char **array = uep->ue_size > 0 ? xcalloc((size_t)uep->ue_size, sizeof(char *)) : NULL;
uep->ue_array = array;
for (size_t i = 0; i < (size_t)uep->ue_size; i++) {
int line_len = undo_read_4c(bi);
char *line;
if (line_len >= 0) {
line = undo_read_string(bi, (size_t)line_len);
} else {
line = NULL;
corruption_error("line length", file_name);
}
int line_len = undo_read_len(bi, "line length", file_name);
char *line = line_len < 0 ? NULL : undo_read_string(bi, (size_t)line_len);
if (line == NULL) {
*error = true;
return uep;
@@ -1453,9 +1455,11 @@ void u_read_undo(char *name, const uint8_t *hash, const char *orig_name FUNC_ATT
goto error;
}
FileInfo file_info;
bufinfo_T bi = {
.bi_buf = curbuf,
.bi_fp = fp,
.bi_fsize = os_fileinfo_fd(fileno(fp), &file_info) ? (off_T)os_fileinfo_size(&file_info) : 0,
};
// Read the undo file header.
@@ -1492,13 +1496,17 @@ void u_read_undo(char *name, const uint8_t *hash, const char *orig_name FUNC_ATT
}
// Read undo data for "U" command.
int str_len = undo_read_4c(&bi);
int str_len = undo_read_len(&bi, "line length", file_name);
if (str_len < 0) {
goto error;
}
if (str_len > 0) {
line_ptr = undo_read_string(&bi, (size_t)str_len);
if (line_ptr == NULL) {
corruption_error("truncated", file_name);
goto error;
}
}
linenr_T line_lnum = (linenr_T)undo_read_4c(&bi);
colnr_T line_colnr = (colnr_T)undo_read_4c(&bi);
@@ -1511,7 +1519,10 @@ void u_read_undo(char *name, const uint8_t *hash, const char *orig_name FUNC_ATT
int old_header_seq = undo_read_4c(&bi);
int new_header_seq = undo_read_4c(&bi);
int cur_header_seq = undo_read_4c(&bi);
int num_head = undo_read_4c(&bi);
int num_head = undo_read_len(&bi, "num_head", file_name);
if (num_head < 0) {
goto error;
}
int seq_last = undo_read_4c(&bi);
int seq_cur = undo_read_4c(&bi);
time_t seq_time = undo_read_time(&bi);
@@ -1543,9 +1554,7 @@ void u_read_undo(char *name, const uint8_t *hash, const char *orig_name FUNC_ATT
// sequence numbers of the headers.
// When there are no headers uhp_table is NULL.
if (num_head > 0) {
if ((size_t)num_head < SIZE_MAX / sizeof(*uhp_table)) {
uhp_table = xmalloc((size_t)num_head * sizeof(*uhp_table));
}
uhp_table = xcalloc((size_t)num_head, sizeof(*uhp_table));
}
int num_read_uhps = 0;
@@ -1739,6 +1748,24 @@ static int undo_read_4c(bufinfo_T *bi)
return get4c(bi->bi_fp);
}
/// Reads a 4-byte count/length field. A corrupted file can hold any value here, so reject negative
/// or if it exceeds the bytes left in the file.
///
/// @param what Name of the field, for the error message.
/// @return The value, or -1 if invalid (reported).
static int undo_read_len(bufinfo_T *bi, const char *what, const char *file_name)
FUNC_ATTR_NONNULL_ALL
{
int len = undo_read_4c(bi);
off_T pos = vim_ftell(bi->bi_fp);
if (len < 0 || (bi->bi_fsize > 0 && (pos < 0 || len > bi->bi_fsize - pos))) {
// get4c() also returns -1 for a file that ends here.
corruption_error(feof(bi->bi_fp) ? "truncated" : what, file_name);
return -1;
}
return len;
}
static int undo_read_2c(bufinfo_T *bi)
{
return get2c(bi->bi_fp);

66
test/functional/editor/undo_spec.lua
View File

@@ -238,3 +238,69 @@ describe("opening file when 'undofile' is on", function()
n.assert_alive()
end)
end)
describe('undo file', function()
before_each(clear)
--- Offset of the first undo entry in `blob`: magic bytes 0xf5 0x18, followed by 4-byte ue_top,
--- ue_bot, ue_lcount, ue_size.
local function find_entry(blob)
for i = 1, #blob - 18 do
if blob:byte(i) == 0xf5 and blob:byte(i + 1) == 0x18 then
local ok = true
-- This expects the ue_xx values to be small, so other data (e.g. a timestamp) containing
-- the magic bytes is not mistaken for an entry.
for field = 0, 3 do
local off = i + 2 + field * 4
ok = ok
and blob:byte(off) == 0
and blob:byte(off + 1) == 0
and blob:byte(off + 2) == 0
and blob:byte(off + 3) <= 8
end
if ok then
return i
end
end
end
end
-- If a corrupted entry is not rejected on load, it would be linked into the undo tree, then
-- u_undoredo() acts on its line numbers and u_freeentry() walks ue_array.
it('rejects corrupted entry (E825)', function()
local txt = t.tmpname()
local undo = txt .. '.undo'
t.write_file(txt, 'one\ntwo\n')
command('edit ' .. txt)
command('set undolevels=100')
command("call setline(1, ['three', 'four'])")
command('write')
command('wundo! ' .. undo)
local blob = assert(t.read_file(undo))
local entry = assert(find_entry(blob))
--- Writes `bytes` over `blob` at `pos` and reads it via :rundo.
local function rundo(pos, bytes)
t.write_file(undo, blob:sub(1, pos - 1) .. bytes .. blob:sub(pos + #bytes))
return pcall_err(command, 'rundo ' .. undo)
end
-- ue_size larger than the file can hold.
for _, bad in ipairs({ '\127\255\255\240', '\255\255\255\255' }) do
eq(
('Vim(rundo):E825: Corrupted undo file (entry size): %s'):format(undo),
rundo(entry + 14, bad)
)
end
-- Negative ue_top/ue_bot/ue_lcount.
for field = 0, 2 do
eq(
('Vim(rundo):E825: Corrupted undo file (entry lnum): %s'):format(undo),
rundo(entry + 2 + field * 4, '\255\255\255\251')
)
end
end)
end)