mirror of
https://github.com/vim/vim.git
synced 2025-09-23 03:43:49 -04:00
patch 9.1.1688: potential buffer overrun in bufwrite.c
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: #18095 Signed-off-by: John Marriott <basilisk@internode.on.net> Signed-off-by: Christian Brabandt <cb@256bit.org>
This commit is contained in:
committed by
Christian Brabandt
parent
2b55474f0a
commit
a19b019b87
@@ -1173,8 +1173,6 @@ buf_write(
|
|||||||
#if defined(UNIX) || defined(MSWIN)
|
#if defined(UNIX) || defined(MSWIN)
|
||||||
else if ((bkc & BKC_AUTO)) // "auto"
|
else if ((bkc & BKC_AUTO)) // "auto"
|
||||||
{
|
{
|
||||||
int i;
|
|
||||||
|
|
||||||
# ifdef UNIX
|
# ifdef UNIX
|
||||||
// Don't rename the file when:
|
// Don't rename the file when:
|
||||||
// - it's a hard link
|
// - it's a hard link
|
||||||
@@ -1201,18 +1199,24 @@ buf_write(
|
|||||||
# endif
|
# endif
|
||||||
# endif
|
# endif
|
||||||
{
|
{
|
||||||
|
size_t dirlen;
|
||||||
|
char_u tmp_fname[MAXPATHL];
|
||||||
|
int i;
|
||||||
|
|
||||||
// Check if we can create a file and set the owner/group to
|
// Check if we can create a file and set the owner/group to
|
||||||
// the ones from the original file.
|
// the ones from the original file.
|
||||||
// First find a file name that doesn't exist yet (use some
|
// First find a file name that doesn't exist yet (use some
|
||||||
// arbitrary numbers).
|
// arbitrary numbers).
|
||||||
STRCPY(IObuff, fname);
|
dirlen = (size_t)(gettail(fname) - fname);
|
||||||
|
vim_strncpy(tmp_fname, fname, dirlen);
|
||||||
fd = -1;
|
fd = -1;
|
||||||
for (i = 4913; ; i += 123)
|
for (i = 4913; ; i += 123)
|
||||||
{
|
{
|
||||||
sprintf((char *)gettail(IObuff), "%d", i);
|
vim_snprintf((char *)tmp_fname + dirlen,
|
||||||
if (mch_lstat((char *)IObuff, &st) < 0)
|
sizeof(tmp_fname) - dirlen, "%d", i);
|
||||||
|
if (mch_lstat((char *)tmp_fname, &st) < 0)
|
||||||
{
|
{
|
||||||
fd = mch_open((char *)IObuff,
|
fd = mch_open((char *)tmp_fname,
|
||||||
O_CREAT|O_WRONLY|O_EXCL|O_NOFOLLOW, perm);
|
O_CREAT|O_WRONLY|O_EXCL|O_NOFOLLOW, perm);
|
||||||
if (fd < 0 && errno == EEXIST)
|
if (fd < 0 && errno == EEXIST)
|
||||||
// If the same file name is created by another
|
// If the same file name is created by another
|
||||||
@@ -1230,7 +1234,7 @@ buf_write(
|
|||||||
# ifdef HAVE_FCHOWN
|
# ifdef HAVE_FCHOWN
|
||||||
vim_ignored = fchown(fd, st_old.st_uid, st_old.st_gid);
|
vim_ignored = fchown(fd, st_old.st_uid, st_old.st_gid);
|
||||||
# endif
|
# endif
|
||||||
if (mch_stat((char *)IObuff, &st) < 0
|
if (mch_stat((char *)tmp_fname, &st) < 0
|
||||||
|| st.st_uid != st_old.st_uid
|
|| st.st_uid != st_old.st_uid
|
||||||
|| st.st_gid != st_old.st_gid
|
|| st.st_gid != st_old.st_gid
|
||||||
|| (long)st.st_mode != perm)
|
|| (long)st.st_mode != perm)
|
||||||
@@ -1239,7 +1243,7 @@ buf_write(
|
|||||||
// Close the file before removing it, on MS-Windows we
|
// Close the file before removing it, on MS-Windows we
|
||||||
// can't delete an open file.
|
// can't delete an open file.
|
||||||
close(fd);
|
close(fd);
|
||||||
mch_remove(IObuff);
|
mch_remove(tmp_fname);
|
||||||
# ifdef MSWIN
|
# ifdef MSWIN
|
||||||
// MS-Windows may trigger a virus scanner to open the
|
// MS-Windows may trigger a virus scanner to open the
|
||||||
// file, we can't delete it then. Keep trying for half a
|
// file, we can't delete it then. Keep trying for half a
|
||||||
@@ -1249,10 +1253,10 @@ buf_write(
|
|||||||
|
|
||||||
for (try = 0; try < 10; ++try)
|
for (try = 0; try < 10; ++try)
|
||||||
{
|
{
|
||||||
if (mch_lstat((char *)IObuff, &st) < 0)
|
if (mch_lstat((char *)tmp_fname, &st) < 0)
|
||||||
break;
|
break;
|
||||||
ui_delay(50L, TRUE); // wait 50 msec
|
ui_delay(50L, TRUE); // wait 50 msec
|
||||||
mch_remove(IObuff);
|
mch_remove(tmp_fname);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
# endif
|
# endif
|
||||||
|
@@ -724,6 +724,8 @@ static char *(features[]) =
|
|||||||
|
|
||||||
static int included_patches[] =
|
static int included_patches[] =
|
||||||
{ /* Add new patch number below this line */
|
{ /* Add new patch number below this line */
|
||||||
|
/**/
|
||||||
|
1688,
|
||||||
/**/
|
/**/
|
||||||
1687,
|
1687,
|
||||||
/**/
|
/**/
|
||||||
|
Reference in New Issue
Block a user