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

Bug 16993: Fix CSRF in memberentry.pl 

If an attacker can get an authenticated Koha user to visit their page
with the url below, they can change patrons' passwords or other
patrons'details

members/memberentry.pl?op=save&destination=circ&borrowernumber=3435&password=ZZZ&password2=ZZZ&nodouble=1

Test plan:

Trigger
members/memberentry.pl?op=save&destination=circ&borrowernumber=42&password=ZZZ&password2=ZZZ&nodouble=1

=> Without this patch, the password will be updated
=> With this patch applied you will get a crash "Wrong CSRF token" (no
need to stylish)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended: removed the commented use Digest::MD5-line.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Jonathan Druart 2016-07-28 12:55:43 +01:00 committed by Kyle M Hall
parent 893f6cc263
commit 09d0b1310b
2 changed files with 23 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
View File

@ -144,12 +144,14 @@ $(document).ready(function() {
<form action="/cgi-bin/koha/members/memberentry.pl" method="get">
<input type="hidden" name="op" value="modify" />
<input type="hidden" name="borrowernumber" value="[% check_member %]" />
<input type="hidden" name="csrf_token" value="[% csrf_token %]" />
<button type="submit" class="new"><i class="fa fa-pencil"></i> It is a duplicate.
Edit existing record</button>
</form>
<form name="form" action="/cgi-bin/koha/members/memberentry.pl" method="post" autocomplete="off">
<input type="hidden" name="nodouble" value="1" />
<input type="hidden" name="csrf_token" value="[% csrf_token %]" />
<button type="submit" class="new"><i class="fa fa-plus"></i> Not a duplicate.
Save as new record</button>
</div>
@ -233,6 +235,7 @@ $(document).ready(function() {
<input type="hidden" name="check_member" value="[% check_member %]" />
<input type="hidden" name="borrowernumber" value="[% UNLESS opduplicate %][% borrowernumber %][% END %]" />
<input type="hidden" name="nodouble" value="[% UNLESS opduplicate %][% nodouble %][% END %]" />
<input type="hidden" name="csrf_token" value="[% csrf_token %]" />
[% IF ( step ) %]<input type="hidden" name="step" value="[% step %]" />[% END %]
[% IF ( opadd ) %]<input type="hidden" name="op" value="insert" />
[% ELSIF ( opduplicate ) %]

22
members/memberentry.pl
View File

@ -24,8 +24,8 @@ use warnings;
# external modules
use CGI qw ( -utf8 );
# use Digest::MD5 qw(md5_base64);
use List::MoreUtils qw/uniq/;
use Digest::MD5 qw(md5_base64);
# internal modules
use C4::Auth;
@ -42,6 +42,7 @@ use C4::Form::MessagingPreferences;
use Koha::Patron::Debarments;
use Koha::Cities;
use Koha::DateUtils;
use Koha::Token;
use Email::Valid;
use Module::Load;
if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
@ -281,6 +282,14 @@ if ( ( defined $newdata{'userid'} && $newdata{'userid'} eq '' ) || $check_Borrow
$debug and warn join "\t", map {"$_: $newdata{$_}"} qw(dateofbirth dateenrolled dateexpiry);
my $extended_patron_attributes = ();
if ($op eq 'save' || $op eq 'insert'){
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'),
});
# If the cardnumber is blank, treat it as null.
$newdata{'cardnumber'} = undef if $newdata{'cardnumber'} =~ /^\s*$/;
@ -679,9 +688,18 @@ $template->param(
category_type =>$category_type,
modify => $modify,
nok => $nok,#flag to know if an error
NoUpdateLogin => $NoUpdateLogin
NoUpdateLogin => $NoUpdateLogin,
);
# Generate CSRF token
$template->param(
csrf_token => Koha::Token->new->generate_csrf(
{ id => C4::Context->userenv->{id},
secret => md5_base64( C4::Context->config('pass') ),
}
),
);
if(defined($data{'flags'})){
$template->param(flags=>$data{'flags'});
}