]> git.vanrenterghem.biz Git - git.ikiwiki.info.git/commitdiff
Fix CSRF attacks against the preferences and edit forms. Closes: #475445
authorJoey Hess <joey@kodama.kitenet.net>
Thu, 10 Apr 2008 21:04:43 +0000 (17:04 -0400)
committerJoey Hess <joey@kodama.kitenet.net>
Thu, 10 Apr 2008 21:04:43 +0000 (17:04 -0400)
The fix involved embedding the session id in the forms, and not allowing the
forms to be submitted if the embedded id does not match the session id.

In the case of the preferences form, if the session id is not embedded,
then the CGI parameters are cleared. This avoids a secondary attack where the
link to the preferences form prefills password or other fields, and
the user hits "submit" without noticing these prefilled values.

In the case of the editpage form, the anonok plugin can allow anyone to edit,
and so I chose not to guard against CSRF attacks against users who are not
logged in. Otherwise, it also embeds the session id and checks it.

For page editing, I assume that the user will notice if content or commit
message is changed because of CGI parameters, and won't blndly hit save page.
So I didn't block those CGI paramters. (It's even possible to use those CGI
parameters, for good, not for evil, I guess..)

The only other CSRF attack I can think of in ikiwiki involves the poll plugin.
It's certianly possible to set up a link that causes the user to unknowingly
vote in a poll. However, the poll plugin is not intended to be used for things
that people would want to attack, since anyone can after all edit the poll page
and fill in any values they like. So this "attack" is ignorable.
(cherry picked from commit 72b5ef2c5fb01751992c9400afe2690da5df611f)

Conflicts:

IkiWiki/CGI.pm
debian/changelog
doc/security.mdwn
po/ikiwiki.pot

IkiWiki/CGI.pm
debian/changelog
doc/security.mdwn
templates/editpage.tmpl

index 1e0ee01eb0a0129230b72fb7bd5111c35e5c9873..2fdd6114cfd5ba1428805c96141e80397b5aa83e 100644 (file)
@@ -298,6 +298,16 @@ sub cgi_prefs ($$) { #{{{
        my $q=shift;
        my $session=shift;
 
+       # The session id is stored on the form and checked to
+       # guard against CSRF.
+       my $sid=$q->param('sid');
+       if (! defined $sid) {
+               $q->delete_all;
+       }
+       elsif ($sid ne $session->id) {
+               error(gettext("Your login session has expired."));
+       }
+
        eval q{use CGI::FormBuilder};
        error($@) if $@;
        my $form = CGI::FormBuilder->new(
@@ -324,7 +334,10 @@ sub cgi_prefs ($$) { #{{{
        my @buttons=("Save Preferences", "Logout", "Cancel");
        
        my $user_name=$session->param("name");
-       $form->field(name => "do", type => "hidden");
+       $form->field(name => "do", type => "hidden", value => "prefs",
+               force => 1);
+       $form->field(name => "sid", type => "hidden", value => $session->id,
+               force => 1);
        $form->field(name => "name", disabled => 1,
                value => $user_name, force => 1);
        $form->field(name => "password", type => "password");
@@ -462,6 +475,8 @@ sub cgi_editpage ($$) { #{{{
        }
 
        $form->field(name => "do", type => 'hidden');
+       $form->field(name => "sid", type => "hidden", value => $session->id,
+               force => 1);
        $form->field(name => "from", type => 'hidden');
        $form->field(name => "rcsinfo", type => 'hidden');
        $form->field(name => "subpage", type => 'hidden');
@@ -591,6 +606,16 @@ sub cgi_editpage ($$) { #{{{
                # save page
                page_locked($page, $session);
                
+               # The session id is stored on the form and checked to
+               # guard against CSRF. But only if the user is logged in,
+               # as anonok can allow anonymous edits.
+               if (defined $session->param("name")) {
+                       my $sid=$q->param('sid');
+                       if (! defined $sid || $sid ne $session->id) {
+                               error(gettext("Your login session has expired."));
+                       }
+               }
+
                my $content=$form->field('editcontent');
 
                $content=~s/\r\n/\n/g;
index d2dbe592db31b1cc469f4d8660a381cee9915cd5..1e6c4a1732f8dfb418ec6f4405488c7ad9fb2c1d 100644 (file)
@@ -1,3 +1,11 @@
+ikiwiki (1.33.5) stable-security; urgency=high
+
+  * Fix CSRF attacks against the preferences and edit forms. The fix involved
+    embedding the session id in the forms, and not allowing the forms to be
+    submitted if the embedded id does not match the session id. Closes: #47544
+
+ -- Joey Hess <joeyh@debian.org>  Thu, 10 Apr 2008 16:49:24 -0400
+
 ikiwiki (1.33.4) stable-security; urgency=high
 
   * htmlscrubber security fix: Block javascript in uris. Closes: #465110
index 723c01863ea9a3bc291464dec7553b6bc91c625a..54da7fcd7912f454de733226cb7e6c7208b84713 100644 (file)
@@ -279,3 +279,94 @@ Various directives that cause one page to be included into another could
 be exploited to DOS the wiki, by causing a loop. Ikiwiki has always guarded
 against this one way or another; the current solution should detect all
 types of loops involving preprocessor directives.
+
+## Online editing of existing css and images
+
+A bug in ikiwiki allowed the web-based editor to edit any file that was in
+the wiki, not just files that are page sources. So an attacker (or a
+genuinely helpful user, which is how the hole came to light) could edit
+files like style.css. It is also theoretically possible that an attacker
+could have used this hole to edit images or other files in the wiki, with
+some difficulty, since all editing would happen in a textarea.
+
+This hole was discovered on 10 Feb 2007 and fixed the same day with the
+release of ikiwiki 1.42. A fix was also backported to Debian etch, as
+version 1.33.1. I recommend upgrading to one of these versions if your wiki
+allows web editing.
+
+## html insertion via title
+
+Missing html escaping of the title contents allowed a web-based editor to
+insert arbitrary html inside the title tag of a page. Since that part of
+the page is not processed by the htmlscrubber, evil html could be injected.
+
+This hole was discovered on 21 March 2007 and fixed the same day (er, hour) 
+with the release of ikiwiki 1.46. A fix was also backported to Debian etch,
+as version 1.33.2. I recommend upgrading to one of these versions if your
+wiki allows web editing or aggregates feeds.
+
+## javascript insertion via meta tags
+
+It was possible to use the meta plugin's meta tags to insert arbitrary
+url contents, which could be used to insert stylesheet information
+containing javascript. This was fixed by sanitising meta tags.
+
+This hole was discovered on 21 March 2007 and fixed the same day
+with the release of ikiwiki 1.47. A fix was also backported to Debian etch,
+as version 1.33.3. I recommend upgrading to one of these versions if your
+wiki can be edited by third parties.
+
+## insufficient checking for symlinks in srcdir path
+
+Ikiwiki did not check if path to the srcdir to contained a symlink. If an
+attacker had commit access to the directories in the path, they could
+change it to a symlink, causing ikiwiki to read and publish files that were
+not intended to be published. (But not write to them due to other checks.)
+
+In most configurations, this is not exploitable, because the srcdir is
+checked out of revision control, but the directories leading up to it are
+not. Or, the srcdir is a single subdirectory of a project in revision
+control (ie, `ikiwiki/doc`), and if the subdirectory were a symlink,
+ikiwiki would still typically not follow it.
+
+There are at least two configurations where this is exploitable:
+
+* If the srcdir is a deeper subdirectory of a project. For example if it is
+  `project/foo/doc`, an an attacker can replace `foo` with a symlink to a
+  directory containing a `doc` directory (not a symlink), then ikiwiki
+  would follow the symlink.
+* If the path to the srcdir in ikiwiki's configuration ended in "/", 
+  and the srcdir is a single subdirectory of a project, (ie,
+  `ikiwiki/doc/`), the srcdir could be a symlink and ikiwiki would not
+  notice.
+
+This security hole was discovered on 26 November 2007 and fixed the same
+day with the release of ikiwiki 2.14. I recommend upgrading to this version
+if your wiki can be committed to by third parties. Alternatively, don't use
+a trailing slash in the srcdir, and avoid the (unusual) configurations that
+allow the security hole to be exploited.
+
+## javascript insertion via uris
+
+The htmlscrubber did not block javascript in uris. This was fixed by adding
+a whitelist of valid uri types, which does not include javascript. 
+([[cve CVE-2008-0809]]) Some urls specifyable by the meta plugin could also
+theoretically have been used to inject javascript; this was also blocked
+([[cve CVE-2008-0808]]).
+
+This hole was discovered on 10 February 2008 and fixed the same day
+with the release of ikiwiki 2.31.1. (And a few subsequent versions..)
+A fix was also backported to Debian etch, as version 1.33.4. I recommend
+upgrading to one of these versions if your wiki can be edited by third
+parties.
+
+## Cross Site Request Forging
+
+Cross Site Request Forging could be used to constuct a link that would
+change a logged-in user's password or other preferences if they clicked on
+the link. It could also be used to construct a link that would cause a wiki
+page to be modified by a logged-in user.
+
+These holes were discovered on 10 April 2008 and fixed the same day with
+the release of ikiwiki 2.42. A fix was also backported to Debian etch, as
+version 1.33.4. I recommend upgrading to one of these versions.
index f63a4b08990f1d2e0f0cd053b5255aec2f29000d..0dbc1bd517faff8fe3703667e453be54255a4997 100644 (file)
@@ -26,6 +26,7 @@ conflict and commit again to save your changes.
 <span><TMPL_VAR INDEXLINK>/ <TMPL_VAR FORM-TITLE></span>
 </div>
 <TMPL_VAR FIELD-DO>
+<TMPL_VAR FIELD-SID>
 <TMPL_VAR FIELD-FROM>
 <TMPL_VAR FIELD-RCSINFO>
 <TMPL_IF NAME="PAGE_SELECT">