gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 17097: Fix CSRF in deletemem.pl 

If an attacker can get an authenticated Koha user to visit their page
with the url below, they can delete patrons details.

  /members/deletemem.pl?member=42

Test plan:

0/ Do not apply any patches
1/ Adapt and hit the url above
=> The patron will be deleted without confirmation
2/ Apply first patch
3/ Hit the url
=> you will get a confirmation page
4/ Hit /members/deletemem.pl?member=42&delete_confirmed=1
=> The patron will be deleted without confirmation
5/ Apply the second patch (this one)
6/ Hit /members/deletemem.pl?member=42&delete_confirmed=1
=> you will get a crash "Wrong CSRF token" (no need to stylish)
7/ Delete a patron from the detail page and confirm the deletion
=> you will be redirected to the patron module home page and the patron
has been deleted

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>
This commit is contained in:
Jonathan Druart 2016-08-09 22:29:25 +01:00 committed by Mason James
parent a0b7acfae1
commit 08ec3cd0d0
2 changed files with 19 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
koha-tmpl/intranet-tmpl/prog/en/modules/members/deletemem.tt
View File

@ -33,6 +33,7 @@
<div class="dialog alert">
<h3>Are you sure you want to delete the patron [% firstname %] [% surname %]? This cannot be undone.</h3>
<form action="/cgi-bin/koha/members/deletemem.pl">
<input type="hidden" name="csrf_token" value="[% csrf_token %]" />
<input type="hidden" name="borrowernumber" value="[% borrowernumber %]"/>
<input type="hidden" name="op" value="delete_confirmed" />
<button type="submit" class="approve"><i class="fa fa-fw fa-check"></i> Yes, delete</button>

19
members/deletemem.pl
View File

@ -25,6 +25,7 @@ use strict;
#use warnings; FIXME - Bug 2505
use CGI qw ( -utf8 );
use Digest::MD5 qw(md5_base64);
use C4::Context;
use C4::Output;
use C4::Auth;
@ -32,6 +33,8 @@ use C4::Members;
use C4::Branch; # GetBranches
use Module::Load;
use Koha::Patron::Images;
use Koha::Token;
if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
load Koha::NorwegianPatronDB, qw( NLMarkForDeletion NLSync );
}
@ -142,9 +145,23 @@ if ( $op eq 'delete_confirm' or $countissues > 0 or $flags->{'CHARGES'} or $is_
}
# This is silly written but reflect the same conditions as above
if ( not $countissues > 0 and not $flags->{CHARGES} ne '' and not $is_guarantor and not $deletelocal == 0 ) {
$template->param( op => 'delete_confirm' );
$template->param(
op => 'delete_confirm',
csrf_token => Koha::Token->new->generate_csrf(
{ id => C4::Context->userenv->{id},
secret => md5_base64( C4::Context->config('pass') ),
}
),
);
}
}elsif ( $op eq 'delete_confirmed' ) {
die "Wrong CSRF token"
unless Koha::Token->new->check_csrf({
id => C4::Context->userenv->{id},
secret => md5_base64( C4::Context->config('pass') ),
token => scalar $input->param('csrf_token'),
});
MoveMemberToDeleted($member);
C4::Members::HandleDelBorrower($member);
DelMember($member);