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)

diff --git a/IkiWiki/CGI.pm b/IkiWiki/CGI.pm
index 89f4f2d732715aa3465ca9b7d181409c469e6d75..1db96f9f284ac24ee395224484517a6d264270ad 100644 (file)
--- 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 428b363b61abe5d13379ed8cb9187c879077c21c..852769f60ea04f2f88a643fbac5645580e6da88f 100644 (file)
--- 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 86f93d717416349f98ace5e58e80e03d18894426..33b8efbed41dcd93124a5442ba515bc7c94f410d 100644 (file)
--- 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);
                        }
                }
        }