From 779fa7c6da03fd3173de4a5c21d5615b83ac3ee4 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 26 May 2016 12:33:13 +0100 Subject: [PATCH] Bug 16591: Fix CSRF in opac-memberentry If an attacker can get an authenticated Koha user to visit their page with the code below, they can update the victim's details to arbitrary values. Test plan: Trigger /cgi-bin/koha/opac-memberentry.pl?action=update&borrower_B_city=HACKED&borrower_firstname=KOHA&borrower_surname=test => Without this patch, the update will be done (or modification request) => With this patch applied you will get a crash "Wrong CSRF token" (no need to stylish) Do some regression tests with this patch applied (Update patron infos) QA note: I am not sure it's useful to create a digest of the DB pass, but just in case... Reported by Alex Middleton at Dionach. Signed-off-by: Chris Cormack Signed-off-by: Katrin Fischer Signed-off-by: Kyle M Hall --- C4/Installer/PerlDependencies.pm | 5 +++++ .../opac-tmpl/bootstrap/en/modules/opac-memberentry.tt | 1 + opac/opac-memberentry.pl | 10 +++++++++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/C4/Installer/PerlDependencies.pm b/C4/Installer/PerlDependencies.pm index c397e498ab..a506615cf0 100644 --- a/C4/Installer/PerlDependencies.pm +++ b/C4/Installer/PerlDependencies.pm @@ -807,6 +807,11 @@ our $PERL_DEPS = { 'required' => '0', 'min_ver' => '0.07' }, + 'WWW::CSRF' => { + usage => 'Core', + required => 1, + min_ver => '1.00', + }, }; 1; diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt index d55796eb99..b70ab9e75c 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt @@ -899,6 +899,7 @@ [% IF OPACPatronDetails %]
+
[% END %] diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index 75ecdc33f9..de157efbf4 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -20,6 +20,7 @@ use Modern::Perl; use CGI qw ( -utf8 ); use Digest::MD5 qw( md5_base64 md5_hex ); use String::Random qw( random_string ); +use WWW::CSRF qw(generate_csrf_token check_csrf_token CSRF_OK); use C4::Auth; use C4::Output; @@ -180,6 +181,10 @@ if ( $action eq 'create' ) { } elsif ( $action eq 'update' ) { + my $borrower = GetMember( borrowernumber => $borrowernumber ); + my $csrf_status = check_csrf_token($borrower->{userid}, md5_base64(C4::Context->config('pass')), scalar $cgi->param('csrf_token')); + die "Wrong CSRF token" unless ($csrf_status == CSRF_OK); + my %borrower = ParseCgiForBorrower($cgi); my %borrower_changes = DelEmptyFields(%borrower); @@ -191,7 +196,8 @@ elsif ( $action eq 'update' ) { $template->param( empty_mandatory_fields => \@empty_mandatory_fields, invalid_form_fields => $invalidformfields, - borrower => \%borrower + borrower => \%borrower, + csrf_token => generate_csrf_token($borrower->{userid}, md5_base64(C4::Context->config('pass'))), ); $template->param( action => 'edit' ); @@ -223,6 +229,7 @@ elsif ( $action eq 'update' ) { action => 'edit', nochanges => 1, borrower => GetMember( borrowernumber => $borrowernumber ), + csrf_token => generate_csrf_token($borrower->{userid}, md5_base64(C4::Context->config('pass'))) ); } } @@ -242,6 +249,7 @@ elsif ( $action eq 'edit' ) { #Display logged in borrower's data borrower => $borrower, guarantor => scalar Koha::Patrons->find($borrowernumber)->guarantor(), hidden => GetHiddenFields( $mandatory, 'modification' ), + csrf_token => generate_csrf_token($borrower->{userid}, md5_base64(C4::Context->config('pass'))) ); if (C4::Context->preference('OPACpatronimages')) { -- 2.39.5