From: Simon McVittie <smcv@debian.org>
Date: Wed, 11 Jan 2017 13:22:03 +0000 (+0000)
Subject: CGI, attachment, passwordauth: harden against repeated parameters
X-Git-Tag: debian/3.20120629.2+deb7u2~34
X-Git-Url: http://git.vanrenterghem.biz/git.ikiwiki.info.git/commitdiff_plain/b1c341777de7304287a02adc8b7b324cab44eb0b?ds=sidebyside;hp=93c590e9e8ae2d9ba1705937d11632f8f7e3a786

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

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