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)
master
Simon McVittie 2017-01-11 13:22:03 +00:00
parent b642cbef80
commit d157a97452
3 changed files with 15 additions and 10 deletions

View File

@ -294,8 +294,9 @@ sub cgi_prefs ($$) {
return; return;
} }
elsif ($form->submitted eq 'Save Preferences' && $form->validate) { elsif ($form->submitted eq 'Save Preferences' && $form->validate) {
if (defined $form->field('email')) { my $email = $form->field('email');
userinfo_set($user_name, 'email', $form->field('email')) || if (defined $email) {
userinfo_set($user_name, 'email', $email) ||
error("failed to set email"); error("failed to set email");
} }

View File

@ -158,8 +158,9 @@ sub formbuilder (@) {
} }
$add.="\n"; $add.="\n";
} }
my $content = $form->field('editcontent');
$form->field(name => 'editcontent', $form->field(name => 'editcontent',
value => $form->field('editcontent')."\n\n".$add, value => $content."\n\n".$add,
force => 1) if length $add; force => 1) if length $add;
} }
@ -222,12 +223,12 @@ sub attachment_store {
$filename=IkiWiki::basename($filename); $filename=IkiWiki::basename($filename);
$filename=~s/.*\\+(.+)/$1/; # hello, windows $filename=~s/.*\\+(.+)/$1/; # hello, windows
$filename=IkiWiki::possibly_foolish_untaint(linkpage($filename)); $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. # Check that the user is allowed to edit the attachment.
my $final_filename= my $final_filename=
linkpage(IkiWiki::possibly_foolish_untaint( linkpage(IkiWiki::possibly_foolish_untaint(
attachment_location($form->field('page')))). attachment_location(scalar $form->field('page')))).
$filename; $filename;
eval { eval {
if (IkiWiki::file_pruned($final_filename)) { if (IkiWiki::file_pruned($final_filename)) {
@ -281,12 +282,12 @@ sub attachments_save {
# Move attachments out of holding directory. # Move attachments out of holding directory.
my @attachments; my @attachments;
my $dir=attachment_holding_location($form->field('page')); my $dir=attachment_holding_location(scalar $form->field('page'));
foreach my $filename (glob("$dir/*")) { foreach my $filename (glob("$dir/*")) {
$filename=Encode::decode_utf8($filename); $filename=Encode::decode_utf8($filename);
next unless -f $filename; next unless -f $filename;
my $destdir=linkpage(IkiWiki::possibly_foolish_untaint( my $destdir=linkpage(IkiWiki::possibly_foolish_untaint(
attachment_location($form->field('page')))); attachment_location(scalar $form->field('page'))));
my $absdestdir=$config{srcdir}."/".$destdir; my $absdestdir=$config{srcdir}."/".$destdir;
my $destfile=IkiWiki::basename($filename); my $destfile=IkiWiki::basename($filename);
my $dest=$absdestdir.$destfile; my $dest=$absdestdir.$destfile;

View File

@ -333,10 +333,12 @@ sub formbuilder (@) {
} }
elsif ($form->submitted eq 'Create Account') { elsif ($form->submitted eq 'Create Account') {
my $email = $form->field('email'); my $email = $form->field('email');
my $password = $form->field('password');
if (IkiWiki::userinfo_setall($user_name, { if (IkiWiki::userinfo_setall($user_name, {
'email' => $email, 'email' => $email,
'regdate' => time})) { 'regdate' => time})) {
setpassword($user_name, $form->field('password')); setpassword($user_name, $password);
$form->field(name => "confirm_password", type => "hidden"); $form->field(name => "confirm_password", type => "hidden");
$form->field(name => "email", type => "hidden"); $form->field(name => "email", type => "hidden");
$form->text(gettext("Account creation successful. Now you can Login.")); $form->text(gettext("Account creation successful. Now you can Login."));
@ -395,8 +397,9 @@ sub formbuilder (@) {
elsif ($form->title eq "preferences") { elsif ($form->title eq "preferences") {
if ($form->submitted eq "Save Preferences" && $form->validate) { if ($form->submitted eq "Save Preferences" && $form->validate) {
my $user_name=$form->field('name'); my $user_name=$form->field('name');
if (defined $form->field("password") && length $form->field("password")) { my $password=$form->field('password');
setpassword($user_name, scalar $form->field('password')); if (defined $password && length $password) {
setpassword($user_name, $password);
} }
} }
} }