Comment 0 for bug 12030

Revision history for this message
In , Javier Fernández-Sanguino (jfs) wrote :

Package: vim
Version: 1:6.3-046+1
Severity: minor
Tags: patch security sid woody sarge

Hi there,

Reviewing vim as part of the security audit the Audit team [1] is
conducting I've found what I believe are some race conditions and symlink
attacks through temporary files in vim. They appear in two scripts which
are not installed in Debian in binary locations (they are installed under
/usr/share/doc/vim/tools/) but are provided with execute permissions.

That's mainly why I'm opening this bug up in Debian's BTS and not
contacting the security team directly although the code is present in all
vim releases in Debian.

These appear in:

1.- the tcltags script (runtime/tools/tcltags):
    (...)
    11 tmp_tagfile=/tmp/${program_name}.$$
    (...)
    130 sed -e "/^!_TAG_FILE_SORTED/s/ [01] / $sorted /"
\
    131 -e "/^!_TAG_FILE_FORMAT/s/ 1 / $format /"
\
    132 $tagfile > $tmp_tagfile

2.- the vimspell script (runtime/tools/vimspell.sh)

     16 OUTFILE=/tmp/vimspell.$$
     17 # if you have "tempfile", use the following line
     18 #OUTFILE=`tempfile`
(...)
     30 spell $SPELL_ARGS $INFILE | sort -u |
     31 awk '
     32 {
     33 printf "syntax match SpellErrors \"\\<%s\\>\"\n", $0 ;
     34 }
     35
     36 END {
     37 printf "highlight link SpellErrors ErrorMsg\n\n" ;
     38 }
     39 ' > $OUTFILE
     40 echo "!rm $OUTFILE" >> $OUTFILE
     41 echo $OUTFILE

Since these are tools that are run from vim, an attacker can get a
good-enough approximation of the PIDs that will be used in these temporary
files and can conduct a symlink attack if these tools are used.

The attached patch should fix both of these issues, I've taken the
approach implemented in vimtutor, but modified it slightly for vimspell as
the temporary file cannot be removed by the script (vim removes it) when
mktemp and tempfile are not avilable, there will still be a race condition
in the script. Since most GNU/Linux and UNIX operating systems seem to
have either one I don't think it's a big issue, however.

Best regards

Javier