Re: Bug#286392: autopoint: Insecure temporary directory usage (fwd)

[Bruno Haible]

Javier Fernández-Sanguino Peña wrote:
> The autopoint script does not protect itself from temporary directory
> attacks. Even though it creates a temporary directory and will abort
> if it exists, the directory itself is not safe (depends on the user's
> umask) and symlink attacks can be used against the directory contents
> through race conditions. For example, consider the possibility of a
> user with an "open" umask that creates file writable by his group,
> a member of the same group could create a CVS directory in autopoint's
> directory and have symlinks from common CVS files there (CVSRoot) to
> other files to force a symlink attack to files the user might not have
> access and belong to the user running the script.
> 
> The attached patch tries to prevent this by using safer umask settings
> when creating the temporary directories.

Thanks a lot for this report. I wouldn't ever have noticed this problem.
I'll use the following patch in gettext 0.14.2.

          Bruno



2005-01-10  Bruno Haible  <address@hidden>

        Security fixes.
        * autopoint.in: Exit if the creation of one of the temporary
        directories fails. Also restrict the access to the temporary cvs_dir.
        Reported by Javier Fernández-Sanguino Peña <address@hidden>.

diff -r -c3 --exclude='*.po*' --exclude='*.info*' --exclude='*.html' 
--exclude=Makefile.in --exclude=aclocal.m4 --exclude=configure 
--exclude='po-*-gen*.[ch]' --exclude='*.o' --exclude='*.lo' --exclude='*.gmo' 
--exclude=ABOUT-NLS --exclude=CVS gettext-cvs/gettext-tools/misc/autopoint.in 
gettext-5/gettext-tools/misc/autopoint.in
*** gettext-cvs/gettext-tools/misc/autopoint.in Sun Jan  9 16:17:29 2005
--- gettext-5/gettext-tools/misc/autopoint.in   Tue Jan 11 00:43:52 2005
***************
*** 349,356 ****
  # - work_dir        directory containing the temporary checkout
  cvs_dir=tmpcvs$$
  work_dir=tmpwrk$$
! mkdir "$cvs_dir"
! mkdir "$work_dir"
  CVSROOT="$srcdir/$cvs_dir"
  export CVSROOT
  unset CVS_CLIENT_LOG
--- 349,370 ----
  # - work_dir        directory containing the temporary checkout
  cvs_dir=tmpcvs$$
  work_dir=tmpwrk$$
! # Use an umask of 077, to avoid attacks that work by overwriting files in the
! # "$CVSROOT"/CVSROOT directory.
! (umask 077 && mkdir "$cvs_dir") || {
!   if test -d "$cvs_dir"; then
!     func_fatal_error "directory $cvs_dir already exists"
!   else
!     func_fatal_error "cannot create directory $cvs_dir"
!   fi
! }
! mkdir "$work_dir" || {
!   if test -d "$work_dir"; then
!     func_fatal_error "directory $work_dir already exists"
!   else
!     func_fatal_error "cannot create directory $work_dir"
!   fi
! }
  CVSROOT="$srcdir/$cvs_dir"
  export CVSROOT
  unset CVS_CLIENT_LOG