Bugzilla – Attachment 106725 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.94 KB, created by
Martin Renvoize (ashimema)
on 2020-07-09 14:58:08 UTC
(
hide
)
Description:
Bug 23634: Prevent non-superlibrarians from editing superlibarian emails
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2020-07-09 14:58:08 UTC
Size:
5.94 KB
patch
obsolete
>From 413e2b09eb9f03c759b3f4de5523636b6a9d2acd 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.inc | 4 ++++ > .../prog/en/modules/members/memberentrygen.tt | 10 +++++++++- > members/memberentry.pl | 10 +++++++++- > 3 files changed, 22 insertions(+), 2 deletions(-) > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/member-alt-address-style.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/member-alt-address-style.inc >index 69fb6649b8..7b3f9bbd70 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/member-alt-address-style.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/member-alt-address-style.inc >@@ -222,7 +222,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 %] >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 9ff3b1a8e7..b65bbfa93f 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt >@@ -712,7 +712,11 @@ legend:hover { > [% END %] > Primary email: > </label> >+ [% IF ( NoUpdateEmail ) %] >+ <input type="text" id="email" name="email" size="45" value="[% patron.email | html %]" disabled="disabled" /> >+ [% ELSE %] > <input type="text" id="email" name="email" size="45" value="[% patron.email | html %]" /> >+ [% END %] > [% IF ( mandatoryemail ) %] > <span class="required">Required</span> > [% END %] >@@ -729,12 +733,16 @@ legend:hover { > [% END %] > Secondary email: > </label> >+ [% IF ( NoUpdateEmail ) %] >+ <input type="text" id="emailpro" name="emailpro" size="45" value="[% patron.emailpro | html %]" disabled="disabled"/> >+ [% ELSE %] > <input type="text" id="emailpro" name="emailpro" size="45" value="[% patron.emailpro | html %]" /> >+ [% END %] > [% IF ( mandatoryemailpro ) %] > <span class="required">Required</span> > [% END %] > </li> >- [% END # /UNLESS noemailpro %] >+ [% END #/UNLESS noemailpro %] > > [% UNLESS nofax %] > <li> >diff --git a/members/memberentry.pl b/members/memberentry.pl >index ec6c08c4c5..eb6d63d208 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -101,6 +101,7 @@ my $step = $input->param('step') || 0; > my @errors; > my $borrower_data; > my $NoUpdateLogin; >+my $NoUpdateEmail; > my $userenv = C4::Context->userenv; > my @messages; > >@@ -170,6 +171,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; > } >@@ -210,7 +216,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; > } >@@ -831,6 +838,7 @@ $template->param( > modify => $modify, > nok => $nok,#flag to know if an error > NoUpdateLogin => $NoUpdateLogin, >+ NoUpdateEmail => $NoUpdateEmail, > ); > > # Generate CSRF token >-- >2.20.1
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