From b1c341777de7304287a02adc8b7b324cab44eb0b Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 11 Jan 2017 13:22:03 +0000 Subject: [PATCH] CGI, attachment, passwordauth: harden against repeated parameters These instances of code similar to OVE-20170111-0001 are not believed to be exploitable, because defined(), length(), setpassword(), userinfo_set() and the binary "." operator all have prototypes that force the relevant argument to be evaluated in scalar context. However, using a safer idiom makes mistakes less likely. --- IkiWiki/CGI.pm | 5 +++-- IkiWiki/Plugin/attachment.pm | 11 ++++++----- IkiWiki/Plugin/passwordauth.pm | 9 ++++++--- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/IkiWiki/CGI.pm b/IkiWiki/CGI.pm index b6923b54f..878869ef1 100644 --- a/IkiWiki/CGI.pm +++ b/IkiWiki/CGI.pm @@ -277,8 +277,9 @@ sub cgi_prefs ($$) { return; } elsif ($form->submitted eq 'Save Preferences' && $form->validate) { - if (defined $form->field('email')) { - userinfo_set($user_name, 'email', $form->field('email')) || + my $email = $form->field('email'); + if (defined $email) { + userinfo_set($user_name, 'email', $email) || error("failed to set email"); } diff --git a/IkiWiki/Plugin/attachment.pm b/IkiWiki/Plugin/attachment.pm index ee015ebdf..ab1929e36 100644 --- a/IkiWiki/Plugin/attachment.pm +++ b/IkiWiki/Plugin/attachment.pm @@ -156,8 +156,9 @@ sub formbuilder (@) { } $add.="\n"; } + my $content = $form->field('editcontent'); $form->field(name => 'editcontent', - value => $form->field('editcontent')."\n\n".$add, + value => $content."\n\n".$add, force => 1) if length $add; } @@ -213,12 +214,12 @@ sub attachment_store { $filename=IkiWiki::basename($filename); $filename=~s/.*\\+(.+)/$1/; # hello, windows $filename=IkiWiki::possibly_foolish_untaint(linkpage($filename)); - my $dest=attachment_holding_location($form->field('page')); + my $dest=attachment_holding_location(scalar $form->field('page')); # Check that the user is allowed to edit the attachment. my $final_filename= linkpage(IkiWiki::possibly_foolish_untaint( - attachment_location($form->field('page')))). + attachment_location(scalar $form->field('page')))). $filename; eval { if (IkiWiki::file_pruned($final_filename)) { @@ -270,13 +271,13 @@ sub attachments_save { # Move attachments out of holding directory. my @attachments; - my $dir=attachment_holding_location($form->field('page')); + my $dir=attachment_holding_location(scalar $form->field('page')); foreach my $filename (glob("$dir/*")) { $filename=Encode::decode_utf8($filename); next unless -f $filename; my $destdir=$config{srcdir}."/". linkpage(IkiWiki::possibly_foolish_untaint( - attachment_location($form->field('page')))); + attachment_location(scalar $form->field('page')))); my $destfile=IkiWiki::basename($filename); my $dest=$destdir.$destfile; unlink($dest); diff --git a/IkiWiki/Plugin/passwordauth.pm b/IkiWiki/Plugin/passwordauth.pm index fe1da764a..84961c51f 100644 --- a/IkiWiki/Plugin/passwordauth.pm +++ b/IkiWiki/Plugin/passwordauth.pm @@ -327,10 +327,12 @@ sub formbuilder (@) { } elsif ($form->submitted eq 'Create Account') { my $email = $form->field('email'); + my $password = $form->field('password'); + if (IkiWiki::userinfo_setall($user_name, { 'email' => $email, 'regdate' => time})) { - setpassword($user_name, $form->field('password')); + setpassword($user_name, $password); $form->field(name => "confirm_password", type => "hidden"); $form->field(name => "email", type => "hidden"); $form->text(gettext("Account creation successful. Now you can Login.")); @@ -389,8 +391,9 @@ sub formbuilder (@) { elsif ($form->title eq "preferences") { 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, scalar $form->field('password')); + my $password=$form->field('password'); + if (defined $password && length $password) { + setpassword($user_name, $password); } } } -- 2.39.2