aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--IkiWiki/Plugin/attachment.pm2
-rw-r--r--IkiWiki/Plugin/comments.pm11
-rw-r--r--IkiWiki/Plugin/editpage.pm2
-rw-r--r--IkiWiki/Plugin/notifyemail.pm2
-rw-r--r--IkiWiki/Plugin/passwordauth.pm4
-rw-r--r--IkiWiki/Plugin/po.pm2
-rw-r--r--IkiWiki/Plugin/rename.pm4
-rw-r--r--debian/changelog5
-rw-r--r--doc/security.mdwn22
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)