diff options
author | Joey Hess <joey@kodama.kitenet.net> | 2008-04-10 16:35:30 -0400 |
---|---|---|
committer | Joey Hess <joey@kodama.kitenet.net> | 2008-04-10 16:35:30 -0400 |
commit | 72b5ef2c5fb01751992c9400afe2690da5df611f (patch) | |
tree | 3a7aab2bc11e330e9e73164e438f01afa3846141 /IkiWiki | |
parent | 609e74bbd83925d2eea797a64620a20f57df75a5 (diff) | |
download | ikiwiki-72b5ef2c5fb01751992c9400afe2690da5df611f.tar ikiwiki-72b5ef2c5fb01751992c9400afe2690da5df611f.tar.gz |
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.
Diffstat (limited to 'IkiWiki')
-rw-r--r-- | IkiWiki/CGI.pm | 35 |
1 files changed, 30 insertions, 5 deletions
diff --git a/IkiWiki/CGI.pm b/IkiWiki/CGI.pm index 470677088..65136a269 100644 --- a/IkiWiki/CGI.pm +++ b/IkiWiki/CGI.pm @@ -161,8 +161,18 @@ sub cgi_prefs ($$) { #{{{ my $session=shift; needsignin($q, $session); - decode_cgi_utf8($q); + + # 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( @@ -193,7 +203,10 @@ sub cgi_prefs ($$) { #{{{ buttons => $buttons); }); - $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 => "email", size => 50, fieldset => "preferences"); $form->field(name => "banned_users", size => 50, fieldset => "admin"); @@ -241,11 +254,11 @@ sub cgi_prefs ($$) { #{{{ sub cgi_editpage ($$) { #{{{ my $q=shift; my $session=shift; - - my @fields=qw(do rcsinfo subpage from page type editcontent comments); - my @buttons=("Save Page", "Preview", "Cancel"); decode_cgi_utf8($q); + + my @fields=qw(do rcsinfo subpage from page type editcontent comments); + my @buttons=("Save Page", "Preview", "Cancel"); eval q{use CGI::FormBuilder}; error($@) if $@; my $form = CGI::FormBuilder->new( @@ -316,6 +329,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'); @@ -474,6 +489,16 @@ sub cgi_editpage ($$) { #{{{ else { # save page check_canedit($page, $q, $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 $exists=-e "$config{srcdir}/$file"; |