Bug#544682: [PATCH] Use mkdtemp if available, prevent writing past end of string otherwise.
James Vega
jamessan at debian.org
Sun Nov 8 20:55:01 UTC 2009
Bram,
vim_tempname, when TEMPDIRS is defined, attempts to create a temp directory as
follows:
for each dir in TEMPDIRS
if dir exists
for i in 1 .. 10000
append vXXXXXX string to dirname
mkdir dirname
if mkdir succeeded, break
else, continue with next i
The problem here is that itmp, which stores the directory name, is only
TEMPNAMELEN bytes long (max of 256). Yet, the loop appends 7 characters
potentially 10,000 times. This blatantly writes past the end of itmp if the
loop runs more than a handful of times.
Exactly this happened in <http://bugs.debian.org/544682>. The fault lies
squarely in smbnetfs for erroneously stating that $TMPDIR, $TMPDIRv667563,
$TMPDIRv667563v66754, etc. were existing directories, but Vim should avoid
crashing in that scenario.
First, the logic for creating a temp directory should take advantage of
existing library functionality. To that end, vim_tempfile will now use
mkdtemp if it is available. This change itself prevents the crash from
happening wherever Vim can use mkdtemp (which should be widely available).
Second, if mkdtemp isn't used, vim_tempfile will overwrite the previous
generated string instead of appending to itmp. I.e., directory names will
progress as $TMPDIRv667563, $TMPDIRv667564, $TMPDIRv667565, etc. instead of
$TMPDIRv667563, $TMPDIRv667563v667564, $TMPDIRv667563v667564v667565. This
is meant as a safe guard for the places, if there are any, where the
TEMPDIRS section of the code is run and mkdtemp is not available.
---
src/config.h.in | 1 +
src/configure.in | 2 +-
src/fileio.c | 68 ++++++++++++++++++++++++++++++++++-------------------
3 files changed, 45 insertions(+), 26 deletions(-)
diff --git a/src/config.h.in b/src/config.h.in
index b603c23..0d39b43 100644
--- a/src/config.h.in
+++ b/src/config.h.in
@@ -157,6 +157,7 @@
#undef HAVE_LSTAT
#undef HAVE_MEMCMP
#undef HAVE_MEMSET
+#undef HAVE_MKDTEMP
#undef HAVE_NANOSLEEP
#undef HAVE_OPENDIR
#undef HAVE_FLOAT_FUNCS
diff --git a/src/configure.in b/src/configure.in
index fa58c6e..f994677 100644
--- a/src/configure.in
+++ b/src/configure.in
@@ -2635,7 +2635,7 @@ fi
dnl Check for functions in one big call, to reduce the size of configure
AC_CHECK_FUNCS(bcmp fchdir fchown fseeko fsync ftello getcwd getpseudotty \
getpwnam getpwuid getrlimit gettimeofday getwd lstat memcmp \
- memset nanosleep opendir putenv qsort readlink select setenv \
+ memset mkdtemp nanosleep opendir putenv qsort readlink select setenv \
setpgid setsid sigaltstack sigstack sigset sigsetjmp sigaction \
sigvec strcasecmp strerror strftime stricmp strncasecmp \
strnicmp strpbrk strtol tgetent towlower towupper iswupper \
diff --git a/src/fileio.c b/src/fileio.c
index 5deb1a3..6285789 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -146,6 +146,7 @@ static int get_mac_fio_flags __ARGS((char_u *ptr));
# endif
#endif
static int move_lines __ARGS((buf_T *frombuf, buf_T *tobuf));
+static void vim_settempdir __ARGS((char_u *tempdir));
#ifdef FEAT_AUTOCMD
static char *e_auchangedbuf = N_("E812: Autocommands changed buffer or buffer name");
#endif
@@ -6986,6 +6987,31 @@ vim_deltempdir()
}
#endif
+ static void
+vim_settempdir(tempdir)
+ char_u * tempdir;
+{
+ char_u *buf;
+
+ /* Directory was created, use this name.
+ * Expand to full path; When using the current
+ * directory a ":cd" would confuse us. */
+ buf = alloc((unsigned)MAXPATHL + 1);
+ if (buf != NULL)
+ {
+ if (vim_FullName(tempdir, buf, MAXPATHL, FALSE) == FAIL)
+ STRCPY(buf, tempdir);
+# ifdef __EMX__
+ if (vim_strchr(buf, '/') != NULL)
+ STRCAT(buf, "/");
+ else
+# endif
+ add_pathsep(buf);
+ vim_tempdir = vim_strsave(buf);
+ vim_free(buf);
+ }
+}
+
/*
* vim_tempname(): Return a unique name that can be used for a temp file.
*
@@ -7027,6 +7053,7 @@ vim_tempname(extra_char)
*/
for (i = 0; i < (int)(sizeof(tempdirs) / sizeof(char *)); ++i)
{
+ size_t itmplen;
/* expand $TMP, leave room for "/v1100000/999999999" */
expand_env((char_u *)tempdirs[i], itmp, TEMPNAMELEN - 20);
if (mch_isdir(itmp)) /* directory exists */
@@ -7040,7 +7067,16 @@ vim_tempname(extra_char)
else
# endif
add_pathsep(itmp);
+ itmplen = STRLEN(itmp);
+# ifdef HAVE_MKDTEMP
+ /* Leave room for filename */
+ STRNCAT(itmp, "vXXXXXX", TEMPNAMELEN - 9);
+ if (mkdtemp(itmp) != NULL)
+ {
+ vim_settempdir(itmp);
+ }
+# else
/* Get an arbitrary number of up to 6 digits. When it's
* unlikely that it already exists it will be faster,
* otherwise it doesn't matter. The use of mkdir() avoids any
@@ -7056,14 +7092,14 @@ vim_tempname(extra_char)
mode_t umask_save;
#endif
- sprintf((char *)itmp + STRLEN(itmp), "v%ld", nr + off);
-# ifndef EEXIST
+ sprintf((char *)itmp + itmplen, "v%ld", nr + off);
+# ifndef EEXIST
/* If mkdir() does not set errno to EEXIST, check for
* existing file here. There is a race condition then,
* although it's fail-safe. */
if (mch_stat((char *)itmp, &st) >= 0)
continue;
-# endif
+# endif
#if defined(UNIX) || defined(VMS)
/* Make sure the umask doesn't remove the executable bit.
* "repl" has been reported to use "177". */
@@ -7075,36 +7111,18 @@ vim_tempname(extra_char)
#endif
if (r == 0)
{
- char_u *buf;
-
- /* Directory was created, use this name.
- * Expand to full path; When using the current
- * directory a ":cd" would confuse us. */
- buf = alloc((unsigned)MAXPATHL + 1);
- if (buf != NULL)
- {
- if (vim_FullName(itmp, buf, MAXPATHL, FALSE)
- == FAIL)
- STRCPY(buf, itmp);
-# ifdef __EMX__
- if (vim_strchr(buf, '/') != NULL)
- STRCAT(buf, "/");
- else
-# endif
- add_pathsep(buf);
- vim_tempdir = vim_strsave(buf);
- vim_free(buf);
- }
+ vim_settempdir(itmp);
break;
}
-# ifdef EEXIST
+# ifdef EEXIST
/* If the mkdir() didn't fail because the file/dir exists,
* we probably can't create any dir here, try another
* place. */
if (errno != EEXIST)
-# endif
+# endif
break;
}
+# endif /* HAVE_MKDTEMP */
if (vim_tempdir != NULL)
break;
}
--
1.6.5.2
--
James
GPG Key: 1024D/61326D40 2003-09-02 James Vega <jamessan at debian.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-vim-maintainers/attachments/20091108/48ed214a/attachment.pgp>
More information about the pkg-vim-maintainers
mailing list