From fc3a6bd1aad41afeb0ee30823bf73796c52b82ae Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 10 Apr 2008 17:04:43 -0400 Subject: [PATCH] Fix CSRF attacks against the preferences and edit forms. Closes: #475445 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 | 27 +++++++++++- debian/changelog | 8 ++++ doc/security.mdwn | 91 +++++++++++++++++++++++++++++++++++++++++ templates/editpage.tmpl | 1 + 4 files changed, 126 insertions(+), 1 deletion(-) diff --git a/IkiWiki/CGI.pm b/IkiWiki/CGI.pm index 1e0ee01eb..2fdd6114c 100644 --- a/IkiWiki/CGI.pm +++ b/IkiWiki/CGI.pm @@ -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; diff --git a/debian/changelog b/debian/changelog index d2dbe592d..1e6c4a173 100644 --- a/debian/changelog +++ b/debian/changelog @@ -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 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 diff --git a/doc/security.mdwn b/doc/security.mdwn index 723c01863..54da7fcd7 100644 --- a/doc/security.mdwn +++ b/doc/security.mdwn @@ -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. diff --git a/templates/editpage.tmpl b/templates/editpage.tmpl index f63a4b089..0dbc1bd51 100644 --- a/templates/editpage.tmpl +++ b/templates/editpage.tmpl @@ -26,6 +26,7 @@ conflict and commit again to save your changes. / + -- 2.26.2