Attachment 90192 Details for Bug 22990 – Bug 22990 - Add CSRF protection to basketheader.pl

[patch] Bug 22990 - Add CSRF protection to basketheader.pl

0001-Bug-22990-Add-CSRF-protection-to-basketheader.pl.patch (text/plain), 6.19 KB, created by Amit Gupta on 2019-05-29 17:23:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Amit Gupta

Created: 2019-05-29 17:23:03 UTC

Size: 6.19 KB

patch

obsolete

>From b2484665d5e635e58d64409437e6c7c3203ce95a Mon Sep 17 00:00:00 2001
>From: Amit Gupta <amit.gupta@informaticsglobal.com>
>Date: Wed, 29 May 2019 22:46:18 +0530
>Subject: [PATCH] Bug 22990 - Add CSRF protection to basketheader.pl
>
>Test plan
>1. Create vendor
>2. Create basket using the form
>
><html>
><body>
><form method="POST" action="http://ip:8080/cgi-bin/koha/acqui/basketheader.pl?booksellerid=1&op=add_form">
><input type="hidden" name="booksellerid" value="1"/>
><input type="hidden" name="basketname" value="CSRFAttack-Test"/>
><input type="hidden" name="billingplace" value="MPL"/>
><input type="hidden" name="deliveryplace" value="MPL"/>
><input type="hidden" name="basketbooksellerid" value="1"/>
><input type="hidden" name="basketnote" value=""/>
><input type="hidden" name="basketbooksellernote" value=""/>
><input type="hidden" name="create_items" value="receiving"/>
><input type="hidden" name="op" value="add_validate"/>
><input type="submit" value="Submit">
></form>
></body>
><html>
>3. You can able to create the basket.
>4. Apply the patch.
>5. Create basket using the form
>6. Now you can get error "The form submission failed (Wrong CSRF token).
>Try to come back, refresh the page, then try again."
>---
> C4/Auth.pm                                         |  2 ++
> C4/Output.pm                                       | 31 +++++++++++++++-------
> acqui/basketheader.pl                              |  2 ++
> .../intranet-tmpl/prog/en/includes/csrf-token.inc  |  1 +
> .../prog/en/modules/acqui/basketheader.tt          |  3 ++-
> 5 files changed, 28 insertions(+), 11 deletions(-)
> create mode 100644 koha-tmpl/intranet-tmpl/prog/en/includes/csrf-token.inc
>
>diff --git a/C4/Auth.pm b/C4/Auth.pm
>index d79ce46..4493716 100644
>--- a/C4/Auth.pm
>+++ b/C4/Auth.pm
>@@ -45,6 +45,7 @@ use List::MoreUtils qw/ any /;
> use Encode qw( encode is_utf8);
> use C4::Auth_with_shibboleth;
> use Net::CIDR;
>+use Koha::Token;
> 
> # use utf8;
> use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS $debug $ldap $cas $caslogout);
>@@ -272,6 +273,7 @@ sub get_template_and_user {
>         $template->param( loggedinusernumber => $borrowernumber ); # FIXME Should be replaced with logged_in_user.borrowernumber
>         $template->param( logged_in_user     => $patron );
>         $template->param( sessionID          => $sessionID );
>+        $template->param( csrf_token => Koha::Token->new->generate_csrf({ session_id => scalar $sessionID }));
> 
>         if ( $in->{'type'} eq 'opac' ) {
>             require Koha::Virtualshelves;
>diff --git a/C4/Output.pm b/C4/Output.pm
>index e8a86cc..a33598d 100644
>--- a/C4/Output.pm
>+++ b/C4/Output.pm
>@@ -33,6 +33,7 @@ use Scalar::Util qw( looks_like_number );
> 
> use C4::Context;
> use C4::Templates;
>+use Koha::Token;
> 
> use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
> 
>@@ -328,19 +329,29 @@ Others will be added here depending on the needs (for instance biblio does not e
> sub output_and_exit_if_error {
>     my ( $query, $cookie, $template, $params ) = @_;
>     my $error;
>-    if ( $params and exists $params->{module} ) {
>-        if ( $params->{module} eq 'members' ) {
>-            my $logged_in_user = $params->{logged_in_user};
>-            my $current_patron = $params->{current_patron};
>-            if ( not $current_patron ) {
>-                $error = 'unknown_patron';
>-            }
>-            elsif( not $logged_in_user->can_see_patron_infos( $current_patron ) ) {
>-                $error = 'cannot_see_patron_infos';
>+    if ($params->{module} eq 'CSRF_TOKEN_CHECK') {
>+        my $csrfcheck = Koha::Token->new->check_csrf( {
>+            session_id => scalar $cookie,
>+            token  => scalar $params->{csrf_token},
>+        });
>+        unless ($csrfcheck) {
>+            $error = 'wrong_csrf_token';
>+        }
>+
>+    } else {
>+        if ( $params and exists $params->{module} ) {
>+            if ( $params->{module} eq 'members' ) {
>+                my $logged_in_user = $params->{logged_in_user};
>+                my $current_patron = $params->{current_patron};
>+                if ( not $current_patron ) {
>+                    $error = 'unknown_patron';
>+                } 
>+                elsif( not $logged_in_user->can_see_patron_infos( $current_patron ) ) {
>+                    $error = 'cannot_see_patron_infos';
>+                }
>             }
>         }
>     }
>-
>     output_and_exit( $query, $cookie, $template, $error ) if $error;
>     return;
> }
>diff --git a/acqui/basketheader.pl b/acqui/basketheader.pl
>index 202f6b9..2d17b07 100755
>--- a/acqui/basketheader.pl
>+++ b/acqui/basketheader.pl
>@@ -142,6 +142,8 @@ if ( $op eq 'add_form' ) {
> #End Edit
> } elsif ( $op eq 'add_validate' ) {
> #we are confirming the changes, save the basket
>+#we are checking CSRF Token. Module CSRF_TOKEN_CHECK indicate for CSRF token checking,
>+    output_and_exit_if_error( $input, $input->cookie('CGISESSID'), $template, { module => 'CSRF_TOKEN_CHECK', csrf_token => $input->param('csrf_token') } );
>     if ( $is_an_edit ) {
>         ModBasketHeader(
>             $basketno,
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/csrf-token.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/csrf-token.inc
>new file mode 100644
>index 0000000..703d4eb
>--- /dev/null
>+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/csrf-token.inc
>@@ -0,0 +1 @@
>+<input type="hidden" name="csrf_token" value="[% csrf_token | html %]" />
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketheader.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketheader.tt
>index f6a89c7..17d14f9 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketheader.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketheader.tt
>@@ -32,13 +32,14 @@
> <div class="main container-fluid">
>     <div class="row">
>         <div class="col-md-10 col-md-offset-1 col-lg-8 col-lg-offset-2">
>-
>+    [% INCLUDE 'blocking_errors.inc' %]
>     [% IF ( add_form ) %]
>         [% IF ( basketno ) %]
>             <h1>Edit basket [% basketname | html %]</h1>
>         [% ELSE %]<h1>Add a basket to [% booksellername | html %]</h1>
>         [% END %]
>     <form name="Aform" action="[% script_name | html %]" method="post" class="validated">
>+        [% INCLUDE 'csrf-token.inc' %]
>         <input type="hidden" name="booksellerid" value="[% booksellerid | html %]" />
>         <fieldset class="rows">
>             <ol>
>-- 
>2.7.4
>

Actions: View | Diff | Splinter Review

Attachments on bug 22990: 90102 | 90192 | 90194 | 90202 | 90204 | 90209 | 90238 | 90239 | 90240 | 95381 | 95382 | 95383 | 95384 | 95385 | 95391 | 95392 | 95393 | 95394 | 95395 | 95396 | 97814 | 122010 | 122011 | 122012 | 122013 | 122014 | 122015 | 122016 | 122023 | 122024 | 122025 | 122026 | 122027 | 122028 | 122029 | 122030 | 122031 | 122046 | 124924 | 124925 | 124926 | 124927 | 124928 | 124929 | 124930 | 124931 | 124932 | 124933 | 124934 | 126091 | 126092 | 126093 | 126094 | 126095 | 126096 | 126097 | 126098 | 126099 | 126100 | 126101 | 133238 | 133239 | 133240 | 133241 | 133242 | 133243 | 133244 | 133245 | 133246 | 133247 | 133248 | 133254 | 133255 | 133256 | 133257 | 133258 | 133422 | 133423 | 133424 | 133425 | 133426 | 133427 | 134848 | 134849 | 134850 | 134851 | 134852 | 134853 | 134854 | 141294 | 141295 | 141296 | 144315 | 144316 | 144317 | 144318 | 144319 | 144320 | 144321 | 144322 | 152626 | 152627 | 152628 | 153394 | 153751 | 153753 | 154044