Moritz Muehlenhoff wrote:
> > 1.19-1 source and binary packages work on stable, and the
> > differences to 1.18.4-2 are all local bugfixes, so I figure it
> > doesn't make any sense to separate bugfixes from bugfixes for a
> > special security fix for stable. Well, we could split out
> > storeBackupSync, though that new script is explicitely marked as
> > experimental.
> 
> Security fixes for stable are typically minimal.

I've extracted the patches from the new upstream version.

> http://cve.mitre.org/cgi-bin/cvename.cgi?name=CAN-2005-3146
> seems fixed by the newly introduced checkDelSymlink() function,
> which was added to ten different places in the code (not all of which
> might be security sensitive, but at least two operate directly
> on temporary files).

This does not eliminate the vulnerability but only shortens the vulnerable
window.  An attacker can still re-create the link between the unlink()
and the open() calls.  The proper action would be to use File::Temp
or something similar.

> I'm not sure about 
> http://cve.mitre.org/cgi-bin/cvename.cgi?name=CAN-2005-3148,
> which would require some more studying of the code.

It's the chown call.

It seems that the old version executed "chown uid gid link" which doesn't
work.  The new version executes "chown -h uid:gid link".  My manpage doesn't
document -h though.

Regards,

        Joey

-- 
Everybody talks about it, but nobody does anything about it!  -- Mark Twain

Please always Cc to me when replying to me on the lists.
diff -u -p -Nr --exclude CVS storebackup-1.18.4.orig/bin/storeBackup.pl 
storebackup-1.18.4/bin/storeBackup.pl
--- storebackup-1.18.4.orig/bin/storeBackup.pl  2004-07-23 05:58:47.000000000 
+0200
+++ storebackup-1.18.4/bin/storeBackup.pl       2005-10-07 09:32:59.000000000 
+0200
@@ -4819,6 +4819,7 @@ sub new
        if ($self->{'debug'} == 0 or $self->{'debug'} == 1)
        {
            local *FILE;
+           &::checkDelSymLink($self->{'tmpfile'}, $prLog, 0x01);
            open(FILE, "> " . $self->{'tmpfile'}) or
                $prLog->print('-kind' => 'E',
                              '-str' =>
@@ -4933,6 +4934,7 @@ sub new
     my $tmpfile = $self->{'tmpDir'} . "/storeBackup-dirs.$$";
     $self->{'tmpfile'} = $tmpfile;
     local *FILE;
+    &::checkDelSymLink($tmpfile, $prLog, 0x01);
     open(FILE, "> $tmpfile") or
        $prLog->print('-kind' => 'E',
                      '-str' => ["cannot open <$tmpfile>, exiting"],
diff -u -p -Nr --exclude CVS storebackup-1.18.4.orig/lib/fileDir.pl 
storebackup-1.18.4/lib/fileDir.pl
--- storebackup-1.18.4.orig/lib/fileDir.pl      2004-07-23 05:58:47.000000000 
+0200
+++ storebackup-1.18.4/lib/fileDir.pl   2005-10-07 09:33:42.000000000 +0200
@@ -28,6 +28,46 @@ require 'forkProc.pl';
 use strict;
 
 
+# checks if a file is a symlink and deletes it if wanted
+# return values (if not exiting):
+#               0: no symlink
+#              -1: found symlink
+############################################################
+sub checkDelSymLink
+{
+    my $file = shift;       # name of the file
+    my $prLog = shift;
+    my $delExit = shift;    # set bits:
+                            #  bit 0:  0 = do not delete
+                            #          1 = delete synlink
+                            #              if not possible, exit
+                            #  bit 1:  0 = do not exit (if exists)
+                            #          1 = exit
+
+    return 0 unless -l $file;
+
+    if ($delExit & 0x02)
+    {
+       $prLog->print('-kind' => 'E',
+                     '-str' =>
+                     ["found symbolic link at <$file>, exiting "],
+                     '-exit' => 1);
+    }
+
+    if ($delExit & 0x01)
+    {
+       $prLog->print('-kind' => 'W',
+                     '-str' => ["unlinking symbolic link <$file>"]);
+       unlink $file or
+           $prLog->print('-kind' => 'E',
+                         '-str' => ["cannot unlink <$file>, exiting"],
+                         '-exit' => 1);
+    }
+
+    return -1;
+}
+
+
 ############################################################
 sub splitFileDir
 {
--- storebackup-1.18.4/bin/storeBackup.pl       2004-07-23 05:58:47.000000000 
+0200
+++ storebackup-1.19/bin/storeBackup.pl 2005-08-12 21:11:18.000000000 +0200
@@ -3164,6 +3183,7 @@
                  ["cannot create <$aktDir>, exiting"],
                  '-exit' => 1)
        unless (mkdir $aktDir);
+    chmod 0755, $aktDir;
     my $chmodDir = $chmodMD5File;
     $chmodDir |= 0100 if $chmodDir & 0400;
     $chmodDir |= 0010 if $chmodDir & 0040;

diff -Nur storebackup-1.18.4.orig/bin/storeBackup.pl 
storebackup-1.19.orig/bin/storeBackup.pl
--- storebackup-1.18.4.orig/bin/storeBackup.pl  2004-07-23 05:58:47.000000000 
+0200
+++ storebackup-1.19.orig/bin/storeBackup.pl    2005-08-12 21:11:18.000000000 
+0200
@@ -3606,7 +3626,7 @@
                # geaendert, sondern die Datei, auf die er verweist.
                # (dann muss lchown genommen werden -> Inkompatibilitaeten!?)
                my $chown = forkProc->new('-exec' => 'chown',
-                                         '-param' => [$uid, $gid,
+                                         '-param' => ['-h', "$uid:$gid",
                                                       "$targetDir/$file"],
                                          '-outRandom' => "$tmpdir/chown-",
                                          '-prLog' => $prLog);

Reply via email to