Bug 18019: Add CSRF protection to authorities-home.pl (op==delete)
authorMarcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tue, 7 Feb 2017 08:09:33 +0000 (09:09 +0100)
committerKyle M Hall <kyle@bywatersolutions.com>
Fri, 31 Mar 2017 13:08:24 +0000 (13:08 +0000)
Without this patch, it is possible to delete authority records with URL
manipulation.
Like: /cgi-bin/koha/authorities/authorities-home.pl?op=delete&authid=[XXX]

Test plan:
[1] Go to Authorities. Search for some authorities (without links).
[2] Delete an authority. Should work.
[3] Apply patch.
[4] Construct an URL like above to delete another authority. Should fail.
    Under Plack this results in an internal server error, the log tells
    you: Wrong CSRF token.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Amended the test plan.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
authorities/authorities-home.pl
koha-tmpl/intranet-tmpl/prog/en/modules/authorities/searchresultlist.tt

index 6e0a8fe..9b3a55e 100755 (executable)
@@ -36,6 +36,7 @@ use C4::Search::History;
 use Koha::Authority::Types;
 use Koha::SearchEngine::Search;
 use Koha::SearchEngine::QueryBuilder;
+use Koha::Token;
 
 my $query = new CGI;
 my $dbh   = C4::Context->dbh;
@@ -58,6 +59,12 @@ if ( $op eq "delete" ) {
             debug           => 1,
         }
     );
+
+    die "Wrong CSRF token" unless Koha::Token->new->check_csrf({
+        session_id => scalar $query->cookie('CGISESSID'),
+        token  => scalar $query->param('csrf_token'),
+    });
+
     &DelAuthority( $authid, 1 );
 
     if ( $query->param('operator') ) {
@@ -111,6 +118,12 @@ if ( $op eq "do_search" ) {
         }
     );
 
+    $template->param(
+        csrf_token => Koha::Token->new->generate_csrf({
+            session_id => scalar $query->cookie('CGISESSID'),
+        }),
+    );
+
     # search history
     if (C4::Context->preference('EnableSearchHistory')) {
         if ( $startfrom == 1) {
index 563c44e..76e4589 100644 (file)
@@ -19,7 +19,8 @@ function confirm_deletion(id) {
           + "&orderby=[% orderby %]"
           + "&value=[% value |url %]"
           + "&startfrom=[% startfrom %]"
-          + "&resultsperpage=[% resultsperpage %]";
+          + "&resultsperpage=[% resultsperpage %]"
+          + "&csrf_token=[% csrf_token %]";
     }
 }