ikiwiki-mass-rebuild: Fix tty hijacking vulnerability by using su. (Once su's related...

diff --git a/debian/changelog b/debian/changelog
index cf3539f1daa36f9ec50db4214a606870698fd08b..2a21cfe276f26e5bfd8191b22c4150086705ad2a 100644 (file)
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,5 +1,8 @@
-ikiwiki (3.20110432) UNRELEASED; urgency=low
+ikiwiki (3.20110608) unstable; urgency=high
 
+  * ikiwiki-mass-rebuild: Fix tty hijacking vulnerability by using su.
+    (Once su's related bug #628843 is fixed.) Thanks, Ludwig Nussel.
+    (CVE-2011-1408)
   * search: Update search page when page.tmpl or searchquery.tmpl are locally
     modified.
 

diff --git a/doc/security.mdwn b/doc/security.mdwn
index f0fcb0cd7d65576ab6437030040dfaa13d4b0f3d..b573b5f2348a3bea028706f1af7d5433b3b0f9a7 100644 (file)
--- a/doc/security.mdwn
+++ b/doc/security.mdwn
@@ -480,10 +480,11 @@ untrusted committers, or have the attachments plugin enabled.
 
 Ludwig Nussel discovered a way for users to hijack root's tty when
 ikiwiki-mass-rebuild was run. Additionally, there was some potential
-for information disclosure via symlinks.
+for information disclosure via symlinks. ([[!cve CVE-2011-1408]])
 
 This hole was disconvered on 8 June 2011 and fixed the same day with
 the release of ikiwiki 3.20110608. Note that the fix is dependant on
-a su that has a similar hole fixed; [[!debbug 628843]] tracks fixing
-the hole in Debian's su. An upgrade is a must for any sites whose
-admins run ikiwiki-mass-rebuild.
+a version of su that has a similar hole fixed; [[!debbug 628843]]
+tracks fixing the hole in Debian's su. An upgrade is a must for any
+sites that have `ikiwiki-update-wikilist` installed suid (not the default),
+and whose admins run `ikiwiki-mass-rebuild`.

diff --git a/ikiwiki-mass-rebuild b/ikiwiki-mass-rebuild
index f13033e7f821d7cbae3a7c761147aa27dedf2b45..bbef4946bb5b76d4aa6f5a291b253ada42309529 100755 (executable)
--- a/ikiwiki-mass-rebuild
+++ b/ikiwiki-mass-rebuild
@@ -2,80 +2,45 @@
 use warnings;
 use strict;
 
-sub supplemental_groups {
-       my $user=shift;
-
-       my @list;
-       while (my @fields=getgrent()) {
-               if (grep { $_ eq $user } split(' ', $fields[3])) {
-                       push @list, $fields[2];
-               }
-       }
+my $etcfile="/etc/ikiwiki/wikilist";
 
-       return @list;
+sub root {
+       $> == 0;
 }
 
-sub samelists {
-       my %a=map { $_ => 1 } split(' ', shift());
-       my %b=map { $_ => 1 } split(' ', shift());
-
-       foreach my $i (keys %b) {
-               if (! exists $a{$i}) {
-                       return 0;
-               }
-       }
-       foreach my $i (keys %a) {
-               if (! exists $b{$i}) {
-                       return 0;
-               }
-       }
-       return 1;
+sub username {
+       (getpwuid($>))[0];
 }
 
 sub processline {
-       my $user=shift;
        my $setup=shift;
        
-       if (! getpwnam("$user")) {
-               print STDERR "warning: user $user does not exist\n";
-               return
-       }
        if (! -f "$setup") {
                print STDERR "warning: $setup does not exist, skipping\n";
                return;
        }
-       print "Processing $setup as user $user ...\n";
-       # su is not used because it passes arguments through the shell,
-       # which is not safe for untrusted setup file names.
-       defined(my $pid = fork) or die "Can’t fork: $!";
-       if (! $pid) {
-               my ($uuid, $ugid) = (getpwnam($user))[2, 3];
-               my $grouplist=join(" ", $ugid, sort {$a <=> $b} $ugid, supplemental_groups($user));
-               if (! samelists(($)=$grouplist), $grouplist)) {
-                       die "failed to set egid $grouplist (got back $))";
-               }
-               $(=$ugid;
-               $<=$uuid;
-               $>=$uuid;
-               if ($< != $uuid || $> != $uuid || $( != $ugid) {
-                       die "failed to drop permissions to $user";
-               }
-               %ENV=(
-                       PATH => $ENV{PATH},
-                       HOME => (getpwnam($user))[7],
-               );
-               exec("ikiwiki", "-setup", $setup, @ARGV);
-               die "failed to run ikiwiki: $!";
+       print "Processing $setup as user ".username()." ...\n";
+       my $ret=system("ikiwiki", "-setup", $setup, @ARGV);
+       if ($ret != 0) {
+               print STDERR "warning: processing $setup failed with code $ret\n";
        }
-       waitpid($pid,0);
-       if ($?) {
-               print STDERR "Processing $setup as user $user failed with code $?\n";
+}
+
+my %users;
+sub processuser {
+       my $user=shift;
+       next if $user=~/^-/ || $users{$user};
+       $users{$user}=1;
+       my $ret=system("su", $user, "-s", "/bin/sh", "-c", "--", "$0 --nonglobal @ARGV");
+       if ($ret != 0) {
+               print STDERR "warning: processing for $user failed with code $ret\n";
        }
 }
 
 sub processlist {
        my $file=shift;
-       my $forceuser=shift;
+
+       return unless -e $file;
 
        my $list;
        open ($list, "<$file") || die "$file: $!";
@@ -84,22 +49,28 @@ sub processlist {
                s/^\s+//;
                s/\s+$//;
                next if /^#/ || ! length;
-
-               if (/^([^\s]+)\s+([^\s]+)$/) {
+               if (/^([-\w]+)\s+([^\s]+)$/) {
                        my $user=$1;
                        my $setup=$2;
-                       if (defined $forceuser && $forceuser ne $user) {
-                               print STDERR "warning: in $file line $., attempt to set user to $user, but user forced to $forceuser. Skipping\n";
+                       if (root()) {
+                               processuser($user);
+                       }
+                       else {
+                               if (username() eq $user) {
+                                       processline($setup);
+                               }
                        }
-                       processline($user, $setup);
                }
-               elsif (/^([^\s]+)$/) {
+               elsif (/^([-\w]+)$/) {
                        my $user=$1;
-                       my $home=(getpwnam($user))[7];
-                       if (defined $home && -d $home) {
-                               my $dotfile="$home/.ikiwiki/wikilist";
-                               if (-e $dotfile) {
-                                       processlist($dotfile, $user);
+                       if (root()) {
+                               processuser($user);
+                       }
+                       else {
+                               my $home=(getpwnam($user))[7];
+                               if (defined $home && -d $home) {
+                                       my $dotfile="$home/.ikiwiki/wikilist";
+                                       processlist($dotfile);
                                }
                        }
                }
@@ -107,9 +78,13 @@ sub processlist {
        close $list;
 }
 
-my $wikilist="/etc/ikiwiki/wikilist";
-
-if (-e $wikilist) {
-       processlist($wikilist);
+if (@ARGV && $ARGV[0] eq "--nonglobal") {
+       shift;
+       # avoid recursively processing if the wikilist file has a root
+       # user in it
+       if (root()) {
+               exit 1;
+       }
 }
 
+processlist($etcfile);