diff options
-rw-r--r-- | IkiWiki/Plugin/attachment.pm | 2 | ||||
-rw-r--r-- | IkiWiki/Plugin/comments.pm | 11 | ||||
-rw-r--r-- | IkiWiki/Plugin/editpage.pm | 2 | ||||
-rw-r--r-- | IkiWiki/Plugin/notifyemail.pm | 2 | ||||
-rw-r--r-- | IkiWiki/Plugin/passwordauth.pm | 4 | ||||
-rw-r--r-- | IkiWiki/Plugin/po.pm | 2 | ||||
-rw-r--r-- | IkiWiki/Plugin/rename.pm | 4 | ||||
-rw-r--r-- | debian/changelog | 5 | ||||
-rw-r--r-- | doc/security.mdwn | 22 |
9 files changed, 41 insertions, 13 deletions
diff --git a/IkiWiki/Plugin/attachment.pm b/IkiWiki/Plugin/attachment.pm index e8135a8fd..428b363b6 100644 --- a/IkiWiki/Plugin/attachment.pm +++ b/IkiWiki/Plugin/attachment.pm @@ -165,7 +165,7 @@ sub formbuilder (@) { # Generate the attachment list only after having added any new # attachments. - $form->tmpl_param("attachment_list" => [attachment_list($form->field('page'))]); + $form->tmpl_param("attachment_list" => [attachment_list(scalar $form->field('page'))]); } sub attachment_holding_location { diff --git a/IkiWiki/Plugin/comments.pm b/IkiWiki/Plugin/comments.pm index b47f965e7..0858f69f1 100644 --- a/IkiWiki/Plugin/comments.pm +++ b/IkiWiki/Plugin/comments.pm @@ -557,11 +557,12 @@ sub editcomment ($$) { } $postcomment=1; - my $ok=IkiWiki::check_content(content => $form->field('editcontent'), - subject => $form->field('subject'), + my $ok=IkiWiki::check_content( + content => scalar $form->field('editcontent'), + subject => scalar $form->field('subject'), $config{comments_allowauthor} ? ( - author => $form->field('author'), - url => $form->field('url'), + author => scalar $form->field('author'), + url => scalar $form->field('url'), ) : (), page => $location, cgi => $cgi, @@ -601,7 +602,7 @@ sub editcomment ($$) { length $form->field('subject')) { $message = sprintf( gettext("Added a comment: %s"), - $form->field('subject')); + scalar $form->field('subject')); } IkiWiki::rcs_add($file); diff --git a/IkiWiki/Plugin/editpage.pm b/IkiWiki/Plugin/editpage.pm index 6ca4b589f..99a142914 100644 --- a/IkiWiki/Plugin/editpage.pm +++ b/IkiWiki/Plugin/editpage.pm @@ -431,7 +431,7 @@ sub cgi_editpage ($$) { $conflict=rcs_commit( file => $file, message => $message, - token => $form->field("rcsinfo"), + token => scalar $form->field("rcsinfo"), session => $session, ); enable_commit_hook(); diff --git a/IkiWiki/Plugin/notifyemail.pm b/IkiWiki/Plugin/notifyemail.pm index b50a22a00..079bb10d4 100644 --- a/IkiWiki/Plugin/notifyemail.pm +++ b/IkiWiki/Plugin/notifyemail.pm @@ -34,7 +34,7 @@ sub formbuilder (@) { } elsif ($form->submitted eq "Save Preferences" && $form->validate && defined $form->field("subscriptions")) { - setsubscriptions($username, $form->field('subscriptions')); + setsubscriptions($username, scalar $form->field('subscriptions')); } } diff --git a/IkiWiki/Plugin/passwordauth.pm b/IkiWiki/Plugin/passwordauth.pm index 3bdd9de2e..c966087ce 100644 --- a/IkiWiki/Plugin/passwordauth.pm +++ b/IkiWiki/Plugin/passwordauth.pm @@ -231,7 +231,7 @@ sub formbuilder_setup (@) { $form->field( name => "password", validate => sub { - checkpassword($form->field("name"), shift); + checkpassword(scalar $form->field("name"), shift); }, ); } @@ -395,7 +395,7 @@ sub formbuilder (@) { if ($form->submitted eq "Save Preferences" && $form->validate) { my $user_name=$form->field('name'); if (defined $form->field("password") && length $form->field("password")) { - setpassword($user_name, $form->field('password')); + setpassword($user_name, scalar $form->field('password')); } } } diff --git a/IkiWiki/Plugin/po.pm b/IkiWiki/Plugin/po.pm index 6b55ee351..418e8e58a 100644 --- a/IkiWiki/Plugin/po.pm +++ b/IkiWiki/Plugin/po.pm @@ -548,7 +548,7 @@ sub formbuilder_setup (@) { # their buttons, which is why this hook must be run last. # The canrename/canremove hooks already ensure this is forbidden # at the backend level, so this is only UI sugar. - if (istranslation($form->field("page"))) { + if (istranslation(scalar $form->field("page"))) { map { for (my $i = 0; $i < @{$params{buttons}}; $i++) { if (@{$params{buttons}}[$i] eq $_) { diff --git a/IkiWiki/Plugin/rename.pm b/IkiWiki/Plugin/rename.pm index 4a86d5a09..56dfbd543 100644 --- a/IkiWiki/Plugin/rename.pm +++ b/IkiWiki/Plugin/rename.pm @@ -259,7 +259,7 @@ sub formbuilder (@) { my $session=$params{session}; if ($form->submitted eq "Rename" && $form->field("do") eq "edit") { - rename_start($q, $session, 0, $form->field("page")); + rename_start($q, $session, 0, scalar $form->field("page")); } elsif ($form->submitted eq "Rename Attachment") { my @selected=map { Encode::decode_utf8($_) } $q->param("attachment_select"); @@ -312,7 +312,7 @@ sub sessioncgi ($$) { # performed in check_canrename later. my $srcfile=IkiWiki::possibly_foolish_untaint($pagesources{$src}) if exists $pagesources{$src}; - my $dest=IkiWiki::possibly_foolish_untaint(titlepage($form->field("new_name"))); + my $dest=IkiWiki::possibly_foolish_untaint(titlepage(scalar $form->field("new_name"))); my $destfile=$dest; if (! $q->param("attachment")) { my $type=$q->param('type'); diff --git a/debian/changelog b/debian/changelog index 86d06bdc6..ccf830b27 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,5 +1,10 @@ ikiwiki (3.20161220) UNRELEASED; urgency=medium + * Security: force CGI::FormBuilder->field to scalar context where + necessary, avoiding unintended function argument injection + analogous to CVE-2014-1572. In ikiwiki this could be used to + forge commit metadata, but thankfully nothing more serious. + (OVE-20161226-0001) * Add CVE references for CVE-2016-10026 * Add missing ikiwiki.setup for the manual test for CVE-2016-10026 * git: don't issue a warning if the rcsinfo CGI parameter is undefined diff --git a/doc/security.mdwn b/doc/security.mdwn index 4f825deba..9818e0c94 100644 --- a/doc/security.mdwn +++ b/doc/security.mdwn @@ -563,3 +563,25 @@ which are both used in most ikiwiki installations. This bug was reported on 2016-12-17. The fixed version 3.20161219 was released on 2016-12-19. ([[!cve CVE-2016-10026]]) + +## Commit metadata forgery via CGI::FormBuilder context-dependent APIs + +When CGI::FormBuilder->field("foo") is called in list context (and +in particular in the arguments to a subroutine that takes named +arguments), it can return zero or more values for foo from the CGI +request, rather than the expected single value. This breaks the usual +Perl parsing convention for named arguments, similar to CVE-2014-1572 +in Bugzilla (which was caused by a similar API design issue in CGI.pm). + +In ikiwiki, this appears to have been exploitable in two places, both +of them relatively minor: + +* in the comments plugin, an attacker who was able to post a comment + could give it a user-specified author and author-URL even if the wiki + configuration did not allow for that, by crafting multiple values + for other fields +* in the editpage plugin, an attacker who was able to edit a page + could potentially forge commit authorship (attribute their edit to + someone else) by crafting multiple values for the rcsinfo field + +(OVE-20161226-0001) |