From 8933e69863a581a799767af4d0a0e1e7b908e238 Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Fri, 25 Aug 2017 15:09:38 -0400 Subject: [PATCH] Bug 18956: Prevent leaking during password recovery TEST PLAN --------- It is assumed you have set the OpacResetPassword to 'allowed', and likely in combination with OpacPasswordChange to 'Allowed'. You will have two patrons: one with and another without any email address entered. You will want to test this test plan with both patrons. $ git checkout -b bug_18956 origin/master Prepend the following as understood between step sections: opac -> forgot password and then enter... correct login/cardnumber, it will email delete from borrower_password_recovery; correct email, it will email delete from borrower_password_recovery; correct login/cardnumber && correct email, it will email delete from borrower_password_recovery; wrong login/cardnumber && correct email, error page as expected delete from borrower_password_recovery; correct login/cardnumber && wrong email, error page as expected delete from borrower_password_recovery; wrong login/cardnumber && wrong email, error page as expected delete from borrower_password_recovery; submit empty -- INTERNAL SERVER ERROR?! delete from borrower_password_recovery; -- None of the above step sections displayed email. correct login/cardnumber, it will email correct login/cardnumber again, but it leaks email address! delete from borrower_password_recovery; correct email, it will email correct email again, but it leaks login/cardnumber! delete from borrower_password_recovery; $ git bz apply 18956 -- choose interactive, and choose this counter patch. repeat the same test set again -- no leaks will occur, error message pages returned should be reasonable, code should read reasonably. run koha qa test tools. Signed-off-by: Marcel de Rooy --- .../bootstrap/en/modules/opac-password-recovery.tt | 7 ++++++- opac/opac-password-recovery.pl | 10 ++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt index 4f3f8571d5..b16a33bc0a 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt @@ -58,7 +58,12 @@ Account identification with this email address only is ambiguous.
Please use the field 'Login' as well. [% ELSIF (errAlreadyStartRecovery) %] - The process of password recovery has already been started for this account ("[% username %]") + The process of password recovery has already been started for this account + [% IF username %] + ("[% username %]") + [% ELSIF email %] + ("[% email %]") + [% END %]
You should have received an email with a link to reset your password.
If you did not receive this email, you can request a new one: Get new password recovery link [% ELSIF (errPassNotMatch) %] diff --git a/opac/opac-password-recovery.pl b/opac/opac-password-recovery.pl index f15a93e2f3..9a7a0ca3ee 100755 --- a/opac/opac-password-recovery.pl +++ b/opac/opac-password-recovery.pl @@ -31,7 +31,7 @@ my $repeatPassword = $query->param('repeatPassword'); my $minPassLength = C4::Context->preference('minPasswordLength'); my $id = $query->param('id'); my $uniqueKey = $query->param('uniqueKey'); -my $username = $query->param('username'); +my $username = $query->param('username') // q{}; my $borrower_number; #errors @@ -53,7 +53,6 @@ my $errPassTooShort; if ( $query->param('sendEmail') || $query->param('resendEmail') ) { #try with the main email - $email ||= ''; # avoid undef my $borrower; my $search_results; @@ -65,7 +64,7 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) { $search_results = Koha::Patrons->search( { -or => { email => $email, emailpro => $email, B_email => $email } } ); } - if ( not $search_results || $search_results->count < 1) { + if ( !defined $search_results || $search_results->count < 1) { $hasError = 1; $errNoBorrowerFound = 1; } @@ -78,7 +77,6 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) { $errMultipleAccountsForEmail = 1; } elsif ( $borrower = $search_results->next() ) { # One matching borrower - $username ||= $borrower->userid; my @emails = ( $borrower->email, $borrower->emailpro, $borrower->B_email ); my $firstNonEmptyEmail = ''; @@ -93,8 +91,8 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) { $errNoBorrowerFound = 1; } -# If we dont have an email yet. Get one of the borrower's email or raise an error. - elsif ( !$email && !( $email = $firstNonEmptyEmail ) ) { + # If there is no given email, and there is no email on record + elsif ( !$email && !$firstNonEmptyEmail ) { $hasError = 1; $errNoBorrowerEmail = 1; } -- 2.39.5