Bug#174987: tetex-bin: xdvi wrapper has a temporary file race condition (security hole)

[Julian Gilbey <jdg@polya.uklinux.net>]

severity 174987 normal
thanks

On Wed, Jan 01, 2003 at 11:24:47PM -0500, Chung-chieh Shan wrote:
> Package: tetex-bin
> Version: 1.0.7+20021025-6
> Severity: grave
> Tags: patch sid
> Justification: user security hole
> 
> Hello,
> 
> The new xdvi wrapper in /usr/bin has the following problems:
> 
>   - The temporary file that compressed files are decompressed into is
>     created in the current working directory.  This creates a race
>     condition and exploitable security hole.

Please see the documentation of File::Temp; this *is* safe.  (And it's
probably better to use a well-tested, documented secure interface like
this than to rewrite our own.)

>   - File names containing an apostrophe or a backslash are not handled
>     for decompression.

Oh.  Good point.  (Yuck.)

>   - If xdvi.bin terminates with a nonzero exit code, the exit code of
>     the xdvi wrapper is not the same exit code but 256 times that code.

Ditto.

>   - If gzip or bzip2 is killed by a signal or dumps core, the xdvi
>     wrapper still proceeds to invoke xdvi.bin.

I don't think that's the case; normally, when such things happen, the
exit status ($? >> 8) is non-zero as well.

Anyhow, I like your patch.  The only change I am making is to include
your name at the top and a "use 5.8;" statement, as the form of the
open command you used was only introduced at this stage.

A full version of the new xdvi is therefore attached; Atsuhito, if you
could include this one in place of the existing one?

Thanks,

   Julian

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

        Julian Gilbey, website: http://www.polya.uklinux.net/
   Debian GNU/Linux Developer, see: http://people.debian.org/~jdg/
     Visit http://www.thehungersite.com/ to help feed the hungry

#! /usr/bin/perl -w

# This is the xdvi wrapper script for Debian, based on Thomas Esser's
# teTeX version 0.2.
# Debian version: Copyright Julian Gilbey, 2002.  Lots of modifications
# by Chung-chieh Shan <ken@digitas.harvard.edu>.
# Original version: Copyright Thomas Esser, 1998.
# Permission to distribute this software is given under the terms of
# the GNU general public license version 2 or later.

# Thomas writes:
# This script sets some environment variables to make sure that xdvi's
# resource file in $XDVIINPUTS/xdvi is read by xdvi.bin.
# Special care was taken to make this work for old R3, too. Therefore,
# we need to modify XAPPLRESDIR. If you are running R4 or later, you
# can set XUSERFILESEARCHPATH for user specific application default
# files. You cannot use XAPPLRESDIR for user specific application default
# files.

# Julian writes:
# While Debian runs X11R6, we keep the XAPPLRESDIR part of the code for
# consistency.  This has been rewritten in Perl so that we can mangle
# the arguments to handled gzipped dvi files, which not have errors if
# there are spaces in some arguments.

use 5.8;  # The temporary file "open" command below was only introduced here
use strict;
use File::Basename;

my @NAMEOPT;
if (@ARGV == 1 and ($ARGV[0] eq '-help' or $ARGV[0] eq '-version')) {
    @NAMEOPT=();
} else {
    @NAMEOPT=qw(-name xdvi);
}

$ENV{'XDVIINPUTS'} .= ":\$TEXMF/{xdvi,web2c}";

my ($xdviappfile, $xdviappdir, $xdviapppath);
$xdviappfile=`kpsewhich -progname=xdvi --format='other text files' XDvi`;
if ("$xdviappfile" ne '') {
    $xdviappdir=dirname($xdviappfile);
    $xdviapppath="$xdviappdir/%N";

    # For R3, we have to set XAPPLRESDIR.
    $ENV{'XAPPLRESDIR'}=$xdviappdir;

    # For R4 or later, we have to set XFILESEARCHPATH, since XAPPLRESDIR might
    # be ignored (if XUSERFILESEARCHPATH is set)
    if (exists $ENV{'XFILESEARCHPATH'}) {
	$ENV{'XFILESEARCHPATH'} = "$xdviapppath:$ENV{'XFILESEARCHPATH'}";
    } else {
	$ENV{'XFILESEARCHPATH'} = $xdviapppath;
    }
}

my $status;
if (@ARGV) {
    my $filename = pop @ARGV;

    if ($filename =~ /\.(gz|Z|bz2)$/) {
	my @command = $1 eq 'bz2' ? qw(bzip2 -d -c) : qw(gzip -d -c);

	require Fcntl;
	open TEMP, "+>", undef
	    or die "xdvi: cannot create temporary file: $!\n";
	fcntl TEMP, Fcntl::F_SETFD(), 0
	    or die "xdvi: disabling close-on-exec for temporary file: $!\n";

	if (my $child = fork) {
	    1 while wait != $child;
	    if ($? & 255) {
		die "xdvi: $command[0] terminated abnormally: $?\n";
	    } elsif ($?) {
		my $code = $? >> 8;
		die "xdvi: $command[0] terminated with exit code $code\n";
	    }
	} elsif (defined $child) {
	    open STDOUT, ">&TEMP";
	    exec @command, $filename;
	} else {
	    die "xdvi: fork: $!\n";
	}
	$status = system('xdvi.bin', @NAMEOPT, @ARGV, "/dev/fd/".fileno(TEMP));
    } else {
	$status = system('xdvi.bin', @NAMEOPT, @ARGV, $filename);
    }
} else {
    $status = system('xdvi.bin', @NAMEOPT);
}

if ($status & 255) {
    die "xdvi: xdvi.bin terminated abnormally: $?\n";
} else {
    my $code = $? >> 8;
    exit $code;
}