urosm
/
ikiwiki
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Force CGI::FormBuilder->field to scalar context where necessary 

CGI::FormBuilder->field has behaviour similar to the CGI.pm misfeature
we avoided in f4ec7b0. Force it into scalar context where it is used
in an argument list.

This prevents two (relatively minor) commit metadata forgery
vulnerabilities:

* 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
  to other fields.
* In the editpage plugin, an attacker who was able to edit a page
  could potentially forge commit authorship by crafting multiple values
  for the rcsinfo field.

The remaining plugins changed in this commit appear to have been
protected by use of explicit scalar prototypes for the called functions,
but have been changed anyway to make them more obviously correct.
In particular, checkpassword() in passwordauth has a known prototype,
so an attacker cannot trick it into treating multiple values of the
name field as being the username, password and field to check for.

OVE-20161226-0001
master
Simon McVittie 2016-12-24 15:03:51 +00:00
parent e193c75b7d
commit c1120bbbe8
9 changed files with 41 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
IkiWiki/Plugin/attachment.pm
View File

@ -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 {

11
IkiWiki/Plugin/comments.pm
View File

@ -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);

2
IkiWiki/Plugin/editpage.pm
View File

@ -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();

2
IkiWiki/Plugin/notifyemail.pm
View File

@ -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'));
}
}

4
IkiWiki/Plugin/passwordauth.pm
View File

@ -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'));
}
}
}

2
IkiWiki/Plugin/po.pm
View File

@ -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 $_) {

4
IkiWiki/Plugin/rename.pm
View File

@ -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');

5
debian/changelog vendored
View File

@ -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

22
doc/security.mdwn
View File

@ -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)