From d157a97452ae0641f87996b6d0f21c9d222cef3d 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. (cherry picked from commit 69230a2220f673c66b5ab875bfc759b32a241c0d) --- 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 89f4f2d73..1db96f9f2 100644 --- a/IkiWiki/CGI.pm +++ b/IkiWiki/CGI.pm @@ -294,8 +294,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 428b363b6..852769f60 100644 --- a/IkiWiki/Plugin/attachment.pm +++ b/IkiWiki/Plugin/attachment.pm @@ -158,8 +158,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; } @@ -222,12 +223,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)) { @@ -281,12 +282,12 @@ 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=linkpage(IkiWiki::possibly_foolish_untaint( - attachment_location($form->field('page')))); + attachment_location(scalar $form->field('page')))); my $absdestdir=$config{srcdir}."/".$destdir; my $destfile=IkiWiki::basename($filename); my $dest=$absdestdir.$destfile; diff --git a/IkiWiki/Plugin/passwordauth.pm b/IkiWiki/Plugin/passwordauth.pm index 86f93d717..33b8efbed 100644 --- a/IkiWiki/Plugin/passwordauth.pm +++ b/IkiWiki/Plugin/passwordauth.pm @@ -333,10 +333,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.")); @@ -395,8 +397,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); } } }