From 6f8a94351b2a9b90bbcd43d40b2be1c950820d07 Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Tue, 15 Aug 2017 16:16:16 +0000 Subject: [PATCH] Bug 18956: Leaking username/cardnumber or email TEST PLAN correct login/cardnumber - emailed. delete from borrower_password_recovery; correct email - emailed. delete from borrower_password_recovery; correct login/cardnumber && correct email -- emailed. delete from borrower_password_recovery; wrong login/cardnumber - error page as expected 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; empty -- INTERNAL SERVER ERROR?! delete from borrower_password_recovery; --- None of these displayed an email correct login/cardnumber - emailed. correct login/cardnumber - error page as expected, BUT --LEAKED EMAIL! delete from borrower_password_recovery; correct email - emailed. correct email - error page as expected, BUT --LEAKED LOGIN/USERID delete from borrower_password_recovery; Now apply the patch, and do all the error pages make sense? Is more information displayed than was entered? Is there an INTERNAL SERVER ERROR? Run qa test tools --- .../bootstrap/en/modules/opac-password-recovery.tt | 7 ++++++- opac/opac-password-recovery.pl | 16 ++++++++++------ 2 files changed, 16 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 4f3f857..b16a33b 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 f4c5e2b..dc18e9a 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,12 +91,18 @@ 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 we dont have an email and one was not provided. + elsif ( !$email && !$firstNonEmptyEmail ) { $hasError = 1; $errNoBorrowerEmail = 1; } +# If we dont have an email and one was provided. + elsif ( !$email && $firstNonEmptyEmail ) { + $hasError = 1; + $errAlreadyStartRecovery = 1; + } + # Check if a password reset already issued for this borrower AND we are not asking for a new email elsif ( not $query->param('resendEmail') ) { if ( ValidateBorrowernumber( $borrower->borrowernumber ) ) { -- 2.1.4