On Thu, Dec 18, 2008 at 10:01:33PM +0200, Niko Tyni wrote:
> Hi release team,
>
> I understand we're not quite in the deep freeze yet, so please consider
> pre-approving or disapproving these changes I'd like to get in lenny:
>
> perl (5.10.0-19) UNRELEASED; urgency=low
> .
> * Downgrade the perl-doc recommendation to a suggestion.
> (Closes: #496770, #442805)
> * Make File::Temp warn on cleaning up the current working directory at
> exit instead of bailing out. (Closes: #479317)
> * Fix $? when dumping core. (Closes: #509041)
> * Fix a memory leak with Scalar::Util::weaken(). (Closes: #506324)
Here's one more: a non-RC security fix I'd like to get in too.
* [SECURITY] "second half of CVE-2007-4829": Archive::Tar no longer
follows symlinks when unpacking. Upstream fix backported by Ubuntu.
(Closes: #509802)
Patch attached. There are a couple of not strictly necessary cosmetic
fixes in there, but I think there's some value to keeping in sync with
what Ubuntu put in their security update first.
Thanks,
--
Niko Tyni nt...@debian.org
[SECURITY] "second half of CVE-2007-4829": Archive::Tar no longer follows
symlinks when unpacking. Upstream fix backported by Ubuntu. (Closes: #509802)
http://rt.cpan.org/Public/Bug/Display.html?id=30380#txn-436899
second half of unpack issue CVE-2007-4829, from 1.39_01 of Archive::Tar
Original patch from Ubuntu version 5.10.0-11.1ubuntu2.2.
diff -uNrp perl-5.10.0~/lib/Archive/Tar.pm perl-5.10.0/lib/Archive/Tar.pm
--- perl-5.10.0~/lib/Archive/Tar.pm 2007-12-18 02:47:07.000000000 -0800
+++ perl-5.10.0/lib/Archive/Tar.pm 2008-12-03 12:56:19.000000000 -0800
@@ -561,26 +561,61 @@ sub _extract_file {
### it's a relative path ###
} else {
- my $cwd = (defined $self->{cwd} ? $self->{cwd} : cwd());
+ my $cwd = (ref $self and defined $self->{cwd})
+ ? $self->{cwd}
+ : cwd();
my @dirs = defined $alt
? File::Spec->splitdir( $dirs ) # It's a local-OS path
: File::Spec::Unix->splitdir( $dirs ); # it's UNIX-style, likely
# straight from the tarball
- ### paths that leave the current directory are not allowed under
- ### strict mode, so only allow it if a user tells us to do this.
if( not defined $alt and
- not $INSECURE_EXTRACT_MODE and
- grep { $_ eq '..' } @dirs
- ) {
- $self->_error(
- q[Entry ']. $entry->full_path .q[' is attempting to leave the
].
- q[current working directory. Not extracting under SECURE ].
- q[EXTRACT MODE]
- );
- return;
- }
+ not $INSECURE_EXTRACT_MODE
+ ) {
+
+ ### paths that leave the current directory are not allowed under
+ ### strict mode, so only allow it if a user tells us to do this.
+ if( grep { $_ eq '..' } @dirs ) {
+
+ $self->_error(
+ q[Entry ']. $entry->full_path .q[' is attempting to leave
].
+ q[the current working directory. Not extracting under ].
+ q[SECURE EXTRACT MODE]
+ );
+ return;
+ }
+
+ ### the archive may be asking us to extract into a symlink. This
+ ### is not sane and a possible security issue, as outlined here:
+ ### https://rt.cpan.org/Ticket/Display.html?id=30380
+ ### https://bugzilla.redhat.com/show_bug.cgi?id=295021
+ ### https://issues.rpath.com/browse/RPL-1716
+ my $full_path = $cwd;
+ for my $d ( @dirs ) {
+ $full_path = File::Spec->catdir( $full_path, $d );
+
+ ### we've already checked this one, and it's safe. Move on.
+ next if ref $self and $self->{_link_cache}->{$full_path};
+
+ if( -l $full_path ) {
+ my $to = readlink $full_path;
+ my $diag = "symlinked directory ($full_path => $to)";
+
+ $self->_error(
+ q[Entry ']. $entry->full_path .q[' is attempting to ].
+ qq[extract to a $diag. This is considered a security ].
+ q[vulnerability and not allowed under SECURE EXTRACT ].
+ q[MODE]
+ );
+ return;
+ }
+
+ ### XXX keep a cache if possible, so the stats become cheaper:
+ $self->{_link_cache}->{$full_path} = 1 if ref $self;
+ }
+ }
+
### '.' is the directory delimiter, of which the first one has to
### be escaped/changed.
@@ -622,7 +657,8 @@ sub _extract_file {
unless ( -d _ ) {
eval { File::Path::mkpath( $dir, 0, 0777 ) };
if( $@ ) {
- $self->_error( qq[Could not create directory '$dir': $...@] );
+ my $fp = $entry->full_path;
+ $self->_error(qq[Could not create directory '$dir' for '$fp':
$...@]);
return;
}
@@ -672,8 +708,13 @@ sub _extract_file {
$self->_make_special_file( $entry, $full ) or return;
}
- utime time, $entry->mtime - TIME_OFFSET, $full or
- $self->_error( qq[Could not update timestamp] );
+ ### only update the timestamp if it's not a symlink; that will change the
+ ### timestamp of the original. This addresses bug #33669: Could not update
+ ### timestamp warning on symlinks
+ if( not -l $full ) {
+ utime time, $entry->mtime - TIME_OFFSET, $full or
+ $self->_error( qq[Could not update timestamp] );
+ }
if( $CHOWN && CAN_CHOWN ) {
chown $entry->uid, $entry->gid, $full or
@@ -707,8 +748,8 @@ sub _make_special_file {
or $fail++;
}
- $err = qq[Making symbolink link from '] . $entry->linkname .
- qq[' to '$file' failed] if $fail;
+ $err = qq[Making symbolic link '$file' to '] .
+ $entry->linkname .q[' failed] if $fail;
} elsif ( $entry->is_hardlink ) {
my $fail;
