Bugzilla – Attachment 61688 Details for
Bug 18124
CSRF tokens must be limited to user's session
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18124: Change the calls to generate and check CSRF tokens
Bug-18124-Change-the-calls-to-generate-and-check-C.patch (text/plain), 16.37 KB, created by
Jonathan Druart
on 2017-03-29 15:36:36 UTC
(
hide
)
Description:
Bug 18124: Change the calls to generate and check CSRF tokens
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2017-03-29 15:36:36 UTC
Size:
16.37 KB
patch
obsolete
>From 972957dff7a334868b9449f1f855bf7062acc4ac Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Wed, 15 Feb 2017 17:14:13 +0100 >Subject: [PATCH] Bug 18124: Change the calls to generate and check CSRF tokens > >The parameter change in Koha::Token should be applied to the calling >scripts. > >Test plan: >Confirm that the different forms of the scripts modified by this patch >still work correctly. > >Test the problematic behavior: >Open 2 tabs with in same user's session, go on the edit patron page >(memberentry.pl). >Log out and log in from the other tab. >Submit the form >=> Wrong CSRF token should be raised > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> >--- > basket/sendbasket.pl | 10 ++-------- > members/deletemem.pl | 13 +++---------- > members/member-flags.pl | 11 ++--------- > members/member-password.pl | 11 ++--------- > members/memberentry.pl | 13 +++---------- > members/moremember.pl | 7 +------ > opac/opac-memberentry.pl | 13 ++++--------- > opac/opac-sendbasket.pl | 11 +++-------- > tools/import_borrowers.pl | 9 ++------- > tools/picture-upload.pl | 11 +++-------- > 10 files changed, 25 insertions(+), 84 deletions(-) > >diff --git a/basket/sendbasket.pl b/basket/sendbasket.pl >index ab29824..47bd1c9 100755 >--- a/basket/sendbasket.pl >+++ b/basket/sendbasket.pl >@@ -20,7 +20,6 @@ use Modern::Perl; > use CGI qw ( -utf8 ); > use Encode qw(encode); > use Carp; >-use Digest::MD5 qw(md5_base64); > use Mail::Sendmail; > use MIME::QuotedPrint; > use MIME::Base64; >@@ -52,8 +51,7 @@ my $dbh = C4::Context->dbh; > > if ( $email_add ) { > die "Wrong CSRF token" unless Koha::Token->new->check_csrf({ >- id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ session_id => scalar $query->cookie('CGISESSID'), > token => scalar $query->param('csrf_token'), > }); > my $email = Koha::Email->new(); >@@ -176,11 +174,7 @@ else { > url => "/cgi-bin/koha/basket/sendbasket.pl", > suggestion => C4::Context->preference("suggestion"), > virtualshelves => C4::Context->preference("virtualshelves"), >- csrf_token => Koha::Token->new->generate_csrf( >- { id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >- } >- ), >+ csrf_token => Koha::Token->new->generate_csrf({ session_id => scalar $query->cookie('CGISESSID'), }), > ); > output_html_with_http_headers $query, $cookie, $template->output; > } >diff --git a/members/deletemem.pl b/members/deletemem.pl >index 9ab49fb..0943994 100755 >--- a/members/deletemem.pl >+++ b/members/deletemem.pl >@@ -25,8 +25,6 @@ use strict; > #use warnings; FIXME - Bug 2505 > > use CGI qw ( -utf8 ); >-use Digest::MD5 qw(md5_base64); >-use Encode qw( encode ); > use C4::Context; > use C4::Output; > use C4::Auth; >@@ -147,19 +145,14 @@ if ( $op eq 'delete_confirm' or $countissues > 0 or $flags->{'CHARGES'} or $is_ > if ( not $countissues > 0 and not $flags->{CHARGES} ne '' and not $is_guarantor and not $deletelocal == 0 ) { > $template->param( > op => 'delete_confirm', >- csrf_token => Koha::Token->new->generate_csrf( >- { id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >- } >- ), >+ csrf_token => Koha::Token->new->generate_csrf({ session_id => scalar $input->cookie('CGISESSID') }), > ); > } > } elsif ( $op eq 'delete_confirmed' ) { > > die "Wrong CSRF token" >- unless Koha::Token->new->check_csrf({ >- id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ unless Koha::Token->new->check_csrf( { >+ session_id => $input->cookie('CGISESSID'), > token => scalar $input->param('csrf_token'), > }); > my $patron = Koha::Patrons->find( $member ); >diff --git a/members/member-flags.pl b/members/member-flags.pl >index d54984a..c188d98 100755 >--- a/members/member-flags.pl >+++ b/members/member-flags.pl >@@ -8,8 +8,6 @@ use strict; > use warnings; > > use CGI qw ( -utf8 ); >-use Digest::MD5 qw(md5_base64); >-use Encode qw( encode ); > use C4::Output; > use C4::Auth qw(:DEFAULT :EditPermissions); > use C4::Context; >@@ -48,8 +46,7 @@ if ($input->param('newflags')) { > > die "Wrong CSRF token" > unless Koha::Token->new->check_csrf({ >- id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ session_id => scalar $input->cookie('CGISESSID'), > token => scalar $input->param('csrf_token'), > }); > >@@ -217,11 +214,7 @@ $template->param( > is_child => ($bor->{'category_type'} eq 'C'), > activeBorrowerRelationship => (C4::Context->preference('borrowerRelationship') ne ''), > RoutingSerials => C4::Context->preference('RoutingSerials'), >- csrf_token => Koha::Token->new->generate_csrf( >- { id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >- } >- ), >+ csrf_token => Koha::Token->new->generate_csrf( { session_id => scalar $input->cookie('CGISESSID'), } ), > ); > > output_html_with_http_headers $input, $cookie, $template->output; >diff --git a/members/member-password.pl b/members/member-password.pl >index 1008cb6..39bdf30 100755 >--- a/members/member-password.pl >+++ b/members/member-password.pl >@@ -20,9 +20,6 @@ use Koha::Token; > > use Koha::Patron::Categories; > >-use Digest::MD5 qw(md5_base64); >-use Encode qw( encode ); >- > my $input = new CGI; > > my $theme = $input->param('theme') || "default"; >@@ -69,8 +66,7 @@ if ( $newpassword && !scalar(@errors) ) { > > die "Wrong CSRF token" > unless Koha::Token->new->check_csrf({ >- id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ session_id => scalar $input->cookie('CGISESSID'), > token => scalar $input->param('csrf_token'), > }); > >@@ -150,10 +146,7 @@ $template->param( > activeBorrowerRelationship => ( C4::Context->preference('borrowerRelationship') ne '' ), > minPasswordLength => $minpw, > RoutingSerials => C4::Context->preference('RoutingSerials'), >- csrf_token => Koha::Token->new->generate_csrf({ >- id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >- }), >+ csrf_token => Koha::Token->new->generate_csrf({ session_id => scalar $input->cookie('CGISESSID'), }), > ); > > if ( scalar(@errors) ) { >diff --git a/members/memberentry.pl b/members/memberentry.pl >index ef0aa90..768b163 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -25,8 +25,6 @@ use warnings; > # external modules > use CGI qw ( -utf8 ); > use List::MoreUtils qw/uniq/; >-use Digest::MD5 qw(md5_base64); >-use Encode qw( encode ); > > # internal modules > use C4::Auth; >@@ -290,8 +288,7 @@ if ($op eq 'save' || $op eq 'insert'){ > > die "Wrong CSRF token" > unless Koha::Token->new->check_csrf({ >- id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ session_id => scalar $input->cookie('CGISESSID'), > token => scalar $input->param('csrf_token'), > }); > >@@ -750,12 +747,8 @@ $template->param( > ); > > # Generate CSRF token >-$template->param( >- csrf_token => Koha::Token->new->generate_csrf( >- { id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >- } >- ), >+$template->param( csrf_token => >+ Koha::Token->new->generate_csrf( { session_id => scalar $input->cookie('CGISESSID'), } ), > ); > > # HouseboundModule data >diff --git a/members/moremember.pl b/members/moremember.pl >index b3e8d76..3e87a2d 100755 >--- a/members/moremember.pl >+++ b/members/moremember.pl >@@ -36,8 +36,6 @@ > use strict; > #use warnings; FIXME - Bug 2505 > use CGI qw ( -utf8 ); >-use Digest::MD5 qw(md5_base64); >-use Encode qw( encode ); > use C4::Context; > use C4::Auth; > use C4::Output; >@@ -279,10 +277,7 @@ my $patron_image = Koha::Patron::Images->find($data->{borrowernumber}); > $template->param( picture => 1 ) if $patron_image; > # Generate CSRF token for upload and delete image buttons > $template->param( >- csrf_token => Koha::Token->new->generate_csrf({ >- id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >- }), >+ csrf_token => Koha::Token->new->generate_csrf({ session_id => $input->cookie('CGISESSID'),}), > ); > > >diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl >index d62d89c..64f9e03 100755 >--- a/opac/opac-memberentry.pl >+++ b/opac/opac-memberentry.pl >@@ -19,7 +19,6 @@ use Modern::Perl; > > use CGI qw ( -utf8 ); > use Digest::MD5 qw( md5_base64 md5_hex ); >-use Encode qw( encode ); > use JSON; > use List::MoreUtils qw( any each_array uniq ); > use String::Random qw( random_string ); >@@ -223,8 +222,7 @@ elsif ( $action eq 'update' ) { > my $borrower = GetMember( borrowernumber => $borrowernumber ); > die "Wrong CSRF token" > unless Koha::Token->new->check_csrf({ >- id => Encode::encode( 'UTF-8', $borrower->{userid} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ session_id => scalar $cgi->cookie('CGISESSID'), > token => scalar $cgi->param('csrf_token'), > }); > >@@ -244,8 +242,7 @@ elsif ( $action eq 'update' ) { > invalid_form_fields => $invalidformfields, > borrower => \%borrower, > csrf_token => Koha::Token->new->generate_csrf({ >- id => Encode::encode( 'UTF-8', $borrower->{userid} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ session_id => scalar $cgi->cookie('CGISESSID'), > }), > ); > $template->param( patron_attribute_classes => GeneratePatronAttributesForm( $borrowernumber, $attributes ) ); >@@ -290,8 +287,7 @@ elsif ( $action eq 'update' ) { > borrower => GetMember( borrowernumber => $borrowernumber ), > patron_attribute_classes => GeneratePatronAttributesForm( $borrowernumber, $attributes ), > csrf_token => Koha::Token->new->generate_csrf({ >- id => Encode::encode( 'UTF-8', $borrower->{userid} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ session_id => scalar $cgi->cookie('CGISESSID'), > }), > ); > } >@@ -305,8 +301,7 @@ elsif ( $action eq 'edit' ) { #Display logged in borrower's data > guarantor => scalar Koha::Patrons->find($borrowernumber)->guarantor(), > hidden => GetHiddenFields( $mandatory, 'modification' ), > csrf_token => Koha::Token->new->generate_csrf({ >- id => Encode::encode( 'UTF-8', $borrower->{userid} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ session_id => scalar $cgi->cookie('CGISESSID'), > }), > ); > >diff --git a/opac/opac-sendbasket.pl b/opac/opac-sendbasket.pl >index c8c5e48..063dd8b 100755 >--- a/opac/opac-sendbasket.pl >+++ b/opac/opac-sendbasket.pl >@@ -22,7 +22,6 @@ use Modern::Perl; > use CGI qw ( -utf8 ); > use Encode qw(encode); > use Carp; >-use Digest::MD5 qw(md5_base64); > use Mail::Sendmail; > use MIME::QuotedPrint; > use MIME::Base64; >@@ -54,8 +53,7 @@ my $dbh = C4::Context->dbh; > > if ( $email_add ) { > die "Wrong CSRF token" unless Koha::Token->new->check_csrf({ >- id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ session_id => scalar $query->cookie('CGISESSID'), > token => scalar $query->param('csrf_token'), > }); > my $email = Koha::Email->new(); >@@ -196,11 +194,8 @@ else { > url => "/cgi-bin/koha/opac-sendbasket.pl", > suggestion => C4::Context->preference("suggestion"), > virtualshelves => C4::Context->preference("virtualshelves"), >- csrf_token => Koha::Token->new->generate_csrf( >- { id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >- } >- ), >+ csrf_token => Koha::Token->new->generate_csrf( >+ { session_id => scalar $query->cookie('CGISESSID'), } ), > ); > output_html_with_http_headers $query, $cookie, $template->output; > } >diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl >index daf7d59..58c2266 100755 >--- a/tools/import_borrowers.pl >+++ b/tools/import_borrowers.pl >@@ -59,8 +59,6 @@ use Text::CSV; > # Ä > > use CGI qw ( -utf8 ); >-use Digest::MD5 qw(md5_base64); >-use Encode qw( encode ); > > my (@errors, @feedback); > my $extended = C4::Context->preference('ExtendedPatronAttributes'); >@@ -112,8 +110,7 @@ $template->param( SCRIPT_NAME => '/cgi-bin/koha/tools/import_borrowers.pl' ); > if ( $uploadborrowers && length($uploadborrowers) > 0 ) { > die "Wrong CSRF token" > unless Koha::Token->new->check_csrf({ >- id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ session_id => scalar $input->cookie('CGISESSID'), > token => scalar $input->param('csrf_token'), > }); > >@@ -391,9 +388,7 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { > > $template->param( > csrf_token => Koha::Token->new->generate_csrf( >- { id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >- } >+ { session_id => scalar $input->cookie('CGISESSID'), } > ), > ); > >diff --git a/tools/picture-upload.pl b/tools/picture-upload.pl >index a3b549e..e74e262 100755 >--- a/tools/picture-upload.pl >+++ b/tools/picture-upload.pl >@@ -25,8 +25,6 @@ use File::Temp; > use File::Copy; > use CGI qw ( -utf8 ); > use GD; >-use Digest::MD5 qw(md5_base64); >-use Encode qw( encode ); > use C4::Context; > use C4::Auth; > use C4::Output; >@@ -88,8 +86,7 @@ if ( ( $op eq 'Upload' ) && $uploadfile ) { > > die "Wrong CSRF token" > unless Koha::Token->new->check_csrf({ >- id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ session_id => scalar $input->cookie('CGISESSID'), > token => scalar $input->param('csrf_token'), > }); > >@@ -176,8 +173,7 @@ elsif ( ( $op eq 'Upload' ) && !$uploadfile ) { > elsif ( $op eq 'Delete' ) { > die "Wrong CSRF token" > unless Koha::Token->new->check_csrf({ >- id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ session_id => scalar $input->cookie('CGISESSID'), > token => scalar $input->param('csrf_token'), > }); > >@@ -195,8 +191,7 @@ if ( $borrowernumber && !%errors && !$template->param('ERRORS') ) { > else { > $template->param( > csrf_token => Koha::Token->new->generate_csrf({ >- id => Encode::encode( 'UTF-8', C4::Context->userenv->{id} ), >- secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), >+ session_id => scalar $input->cookie('CGISESSID'), > }), > ); > output_html_with_http_headers $input, $cookie, $template->output; >-- >2.9.3
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18124
:
60266
|
60326
|
60332
|
60333
|
60334
|
61282
|
61283
|
61284
| 61688