From 4263ec21c238b2a3da7b1ea711c415827bf63042 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Wed, 27 Aug 2025 10:16:59 +0800 Subject: [PATCH] vim-patch:9.1.1688: potential buffer overrun in bufwrite.c (#35497) Problem: potential buffer overrun in bufwrite.c Solution: Use a temporary variable (John Marriott) In my Windows 11 Pro 64-bit build MAXPATHL is 1024 and IOSIZE is 1025. In my Archlinux Linux 64-bit build MAXPATHL is 4096 and IOSIZE is 1025. In funuction buf_write(): There is a check (line 713) that makes sure the length of fname is less than MAXPATHL. There is a call to STRCPY() (line 1208) which copies the string at fname into IObuff (which has size IOSIZE). For Unix builds fname is set to sfname which may or may not be shorter. However, if sfname is NULL sfname is set to fname. Therefore, in builds where MAXPATHL > IOSIZE (eg in my linux build), it is theoretically possible for the STRCPY() call to exceed the bounds of IObuff. This PR addresses this by copying fname into a local variable that has the same maximum size as fname. In addition: Given that the filename is unconditionally overwritten in the for loop, only copy the directory portion of fname. Move variable i closer to where it is used. closes: vim/vim#18095 https://github.com/vim/vim/commit/a19b019b8745f1c91e42c2b9440bddebfa67a17a Co-authored-by: John Marriott --- src/nvim/bufwrite.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/nvim/bufwrite.c b/src/nvim/bufwrite.c index 4e961c64c7..e0b114b83a 100644 --- a/src/nvim/bufwrite.c +++ b/src/nvim/bufwrite.c @@ -738,23 +738,24 @@ static int buf_write_make_backup(char *fname, bool append, FileInfo *file_info_o // the ones from the original file. // First find a file name that doesn't exist yet (use some // arbitrary numbers). - xstrlcpy(IObuff, fname, IOSIZE); + size_t dirlen = (size_t)(path_tail(fname) - fname); + assert(dirlen < MAXPATHL); + char tmp_fname[MAXPATHL]; + xmemcpyz(tmp_fname, fname, dirlen); for (int i = 4913;; i += 123) { - char *tail = path_tail(IObuff); - size_t size = (size_t)(tail - IObuff); - snprintf(tail, IOSIZE - size, "%d", i); - if (!os_fileinfo_link(IObuff, &file_info)) { + snprintf(tmp_fname + dirlen, sizeof(tmp_fname) - dirlen, "%d", i); + if (!os_fileinfo_link(tmp_fname, &file_info)) { break; } } - int fd = os_open(IObuff, + int fd = os_open(tmp_fname, O_CREAT|O_WRONLY|O_EXCL|O_NOFOLLOW, perm); if (fd < 0) { // can't write in directory *backup_copyp = true; } else { #ifdef UNIX os_fchown(fd, (uv_uid_t)file_info_old->stat.st_uid, (uv_gid_t)file_info_old->stat.st_gid); - if (!os_fileinfo(IObuff, &file_info) + if (!os_fileinfo(tmp_fname, &file_info) || file_info.stat.st_uid != file_info_old->stat.st_uid || file_info.stat.st_gid != file_info_old->stat.st_gid || (int)file_info.stat.st_mode != perm) { @@ -764,7 +765,7 @@ static int buf_write_make_backup(char *fname, bool append, FileInfo *file_info_o // Close the file before removing it, on MS-Windows we // can't delete an open file. close(fd); - os_remove(IObuff); + os_remove(tmp_fname); } } }