found & fixed another symlink attack

diff --git a/doc/security.mdwn b/doc/security.mdwn
index 63d140ec50c4d054911b39169abc2ea748a0b56f..c7a6fcd69dd0d91df051d793ce2d1a0bd2299a52 100644 (file)
--- a/doc/security.mdwn
+++ b/doc/security.mdwn
@@ -141,6 +141,22 @@ into the repo. ikiwiki uses File::Find to traverse the repo, and does not
 tell it to follow symlinks, but it might be possible to race replacing a
 directory with a symlink and trick it into following the link.
 
 tell it to follow symlinks, but it might be possible to race replacing a
 directory with a symlink and trick it into following the link.
 

-Also, if someone checks in a symlink to /etc/passwd, ikiwiki would read and publish that, which could be used to expose files a committer otherwise wouldn't see.
+Also, if someone checks in a symlink to /etc/passwd, ikiwiki would read and
+publish that, which could be used to expose files a committer otherwise
+wouldn't see.

 
 

-To avoid this, ikiwiki will avoid reading files that are symlinks, and uses locking to prevent more than one instance running at a time. The lock prevents one ikiwiki from running a svn up at the wrong time to race another ikiwiki. So only attackers who can write to the working copy on their own can race it.
+To avoid this, ikiwiki will skip over symlinks when scanning for pages, and
+uses locking to prevent more than one instance running at a time. The lock
+prevents one ikiwiki from running a svn up at the wrong time to race
+another ikiwiki. So only attackers who can write to the working copy on
+their own can race it.
+
+## symlink + cgi attacks
+
+Similarly, a svn commit of a symlink could be made, ikiwiki ignores it
+because of the above, but the symlink is still there, and then you edit the
+page from the web, which follows the symlink when reading the page, and
+again when saving the changed page.
+
+This was fixed by making ikiwiki refuse to read or write to files that are
+symlinks, combined with the above locking.

diff --git a/ikiwiki b/ikiwiki
index 78aa65ce24c8c32314dea0fbe82afe9e8d45067b..6b8a5153512b3a2e83bc4f4194154ce7a4f7a56e 100755 (executable)
--- a/ikiwiki
+++ b/ikiwiki
@@ -152,6 +152,10 @@ sub htmlpage ($) { #{{{
 sub readfile ($) { #{{{
        my $file=shift;
 
 sub readfile ($) { #{{{
        my $file=shift;
 

+       if (-l $file) {
+               error("cannot read a symlink ($file)");
+       }
+       

        local $/=undef;
        open (IN, "$file") || error("failed to read $file: $!");
        my $ret=<IN>;
        local $/=undef;
        open (IN, "$file") || error("failed to read $file: $!");
        my $ret=<IN>;


@@ -162,6 +166,10 @@ sub readfile ($) { #{{{
 sub writefile ($$) { #{{{
        my $file=shift;
        my $content=shift;
 sub writefile ($$) { #{{{
        my $file=shift;
        my $content=shift;

+       
+       if (-l $file) {
+               error("cannot write to a symlink ($file)");
+       }

 
        my $dir=dirname($file);
        if (! -d $dir) {
 
        my $dir=dirname($file);
        if (! -d $dir) {


@@ -1334,7 +1342,7 @@ sub cgi_editpage ($$) { #{{{
                            ! length $form->field('content')) {
                                my $content="";
                                if (exists $pagesources{lc($page)}) {
                            ! length $form->field('content')) {
                                my $content="";
                                if (exists $pagesources{lc($page)}) {

-                                               $content=readfile("$config{srcdir}/$pagesources{lc($page)}");
+                                       $content=readfile("$config{srcdir}/$pagesources{lc($page)}");

                                        $content=~s/\n/\r\n/g;
                                }
                                $form->field(name => "content", value => $content,
                                        $content=~s/\n/\r\n/g;
                                }
                                $form->field(name => "content", value => $content,