From 09d0b1310bda677b6939b59ea8a68f84e2ec93f6 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 28 Jul 2016 12:55:43 +0100 Subject: [PATCH] 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 Amended: removed the commented use Digest::MD5-line. Signed-off-by: Katrin Fischer Signed-off-by: Kyle M Hall --- .../prog/en/modules/members/memberentrygen.tt | 3 +++ members/memberentry.pl | 22 ++++++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) 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 20725bd8c8..efc0587f05 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -144,12 +144,14 @@ $(document).ready(function() {
+
+ @@ -233,6 +235,7 @@ $(document).ready(function() { + [% IF ( step ) %][% END %] [% IF ( opadd ) %] [% ELSIF ( opduplicate ) %] diff --git a/members/memberentry.pl b/members/memberentry.pl index 32b2402258..413db97a74 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -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'}); } -- 2.11.0