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 <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/Installer/PerlDependencies.pm b/C4/Installer/PerlDependencies.pm
index c397e49..a506615 100644 (file)
--- 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 d55796e..b70ab9e 100644 (file)
--- 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 %]
                         <fieldset class="action">
                             <input type="hidden" name="action" value="update" />
+                            <input type="hidden" name="csrf_token" value="[% csrf_token %]" />
                             <input type="submit" class="btn" value="Submit update request" />
                         </fieldset>
                     [% END %]

diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl
index 75ecdc3..de157ef 100755 (executable)
--- 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')) {