Bugzilla – Attachment 97987 Details for
Bug 23634
Privilege escalation vulnerability for staff users with 'edit_borrowers' permission and 'OpacResetPassword' enabled
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 23634: Prevent non-superlibrarians from editing superlibarian emails
Bug-23634-Prevent-non-superlibrarians-from-editing.patch (text/plain), 5.12 KB, created by
Tomás Cohen Arazi (tcohen)
on 2020-01-27 15:54:57 UTC
(
hide
)
Description:
Bug 23634: Prevent non-superlibrarians from editing superlibarian emails
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2020-01-27 15:54:57 UTC
Size:
5.12 KB
patch
obsolete
>From 996579c821356c07223bb4278dc363ed8bc357f2 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Tue, 19 Nov 2019 14:51:50 +0000 >Subject: [PATCH] Bug 23634: Prevent non-superlibrarians from editing > superlibarian emails > >This patchset prevents a non-superlibrarian user from editing a >superlibrarians email address via memberentry. This is to prevent a >privilege escalation vulnerability whereby a user could update a >superlibrarians contact details to match their own and then request a >password reset via the OPAC. > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > .../prog/en/includes/member-alt-address-style-us.inc | 4 ++++ > .../prog/en/modules/members/memberentrygen.tt | 12 ++++++++++-- > members/memberentry.pl | 10 +++++++++- > 3 files changed, 23 insertions(+), 3 deletions(-) > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/member-alt-address-style-us.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/member-alt-address-style-us.inc >index ec3d6e274c..97c494e4ae 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/member-alt-address-style-us.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/member-alt-address-style-us.inc >@@ -141,7 +141,11 @@ > <label for="B_email"> > [% END %] > Email: </label> >+ [% IF ( NoUpdateEmail ) %] >+ <input type="text" id="B_email" name="B_email" size="45" value="[% patron.B_email | html %]" disabled="disabled" /> >+ [% ELSE %] > <input type="text" id="B_email" name="B_email" size="45" value="[% patron.B_email | html %]" /> >+ [% END %] > [% IF ( mandatoryB_email ) %]<span class="required">Required</span>[% END %] </li> > [% END %] > [% UNLESS nocontactnote %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >index 23a32e0785..57da07cf47 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >@@ -552,7 +552,11 @@ > <label for="email"> > [% END %] > Primary email: </label> >- <input type="text" id="email" name="email" size="45" value="[% email | html %]" /> >+ [% IF ( NoUpdateEmail ) %] >+ <input type="text" id="email" name="email" size="45" value="[% email | html %]" disabled="disabled" /> >+ [% ELSE %] >+ <input type="text" id="email" name="email" size="45" value="[% email | html %]" /> >+ [% END %] > [% IF ( mandatoryemail ) %]<span class="required">Required</span>[% END %]<div class="hint">Shows on transit slips</div> > </li> > [% END %] >@@ -565,7 +569,11 @@ > <label for="emailpro"> > [% END %] > Secondary email: </label> >- <input type="text" id="emailpro" name="emailpro" size="45" value="[% emailpro | html %]" /> >+ [% IF ( NoUpdateEmail ) %] >+ <input type="text" id="emailpro" name="emailpro" size="45" value="[% emailpro | html %]" disabled="disabled"/> >+ [% ELSE %] >+ <input type="text" id="emailpro" name="emailpro" size="45" value="[% emailpro | html %]" /> >+ [% END %] > [% IF ( mandatoryemailpro ) %]<span class="required">Required</span>[% END %] > </li> > [% END %] >diff --git a/members/memberentry.pl b/members/memberentry.pl >index 902beeb0e8..d0eec70911 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -102,6 +102,7 @@ my $step = $input->param('step') || 0; > my @errors; > my $borrower_data; > my $NoUpdateLogin; >+my $NoUpdateEmail; > my $userenv = C4::Context->userenv; > > ## Deal with guarantor stuff >@@ -166,6 +167,11 @@ if ( $op eq 'modify' or $op eq 'save' or $op eq 'duplicate' ) { > my $logged_in_user = Koha::Patrons->find( $loggedinuser ); > output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); > >+ # check permission to modify email info. >+ if ( $patron->is_superlibrarian && !$logged_in_user->is_superlibrarian ) { >+ $NoUpdateEmail = 1; >+ } >+ > $borrower_data = $patron->unblessed; > $borrower_data->{category_type} = $patron->category->category_type; > } >@@ -206,7 +212,8 @@ if ( $op eq 'insert' || $op eq 'modify' || $op eq 'save' || $op eq 'duplicate' ) > push(@errors,"ERROR_$_"); > } > } >- # check permission to modify login info. >+ >+ # check permission to modify login info. > if (ref($borrower_data) && ($borrower_data->{'category_type'} eq 'S') && ! (C4::Auth::haspermission($userenv->{'id'},{'staffaccess'=>1})) ) { > $NoUpdateLogin = 1; > } >@@ -795,6 +802,7 @@ $template->param( > modify => $modify, > nok => $nok,#flag to know if an error > NoUpdateLogin => $NoUpdateLogin, >+ NoUpdateEmail => $NoUpdateEmail, > ); > > # Generate CSRF token >-- >2.25.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 23634
:
92932
|
92998
|
95563
|
95564
|
95574
|
95575
|
95576
|
95577
|
95582
|
97986
|
97987
|
97988
|
97989
|
97990
|
106159
|
106160
|
106161
|
106162
|
106163
|
106723
|
106724
|
106725
|
106726
|
106727
|
106764
|
106826
|
106837
|
106838
|
106839
|
106840
|
106841
|
106842
|
106843
|
106844
|
106979
|
106980
|
106981
|
106982
|
106983
|
106984
|
106985
|
106986
|
109582