From 11bf7e7bef856d5d90126c19f897d060cb4c9d9d Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 18 Aug 2016 15:52:38 +0100 Subject: [PATCH] Bug 17146: Fix CSRF in picture-upload.pl If an attacker can get an authenticated Koha user to visit their page with the url below, they can change or delete patrons' images /tools/picture-upload.pl?op=Delete&borrowernumber=42 Test plan: 1/ Hit /tools/picture-upload.pl?op=Delete&borrowernumber=42 And confirm that you get a "Wrong CSRF token" error 2/ Go on the patron detail page with a patron's image 3/ Click on the Delete link (note the csrf_token param) 4/ The image will be deleted and you are redirected to the patron detail page. Regression tests: Upload an image from the patron detail page and from the "upload patron images" tool. Signed-off-by: Chris Cormack Signed-off-by: Marcel de Rooy Signed-off-by: Kyle M Hall --- .../prog/en/modules/members/moremember.tt | 3 ++- .../prog/en/modules/tools/picture-upload.tt | 1 + members/moremember.pl | 10 ++++++++++ tools/picture-upload.pl | 16 ++++++++++++++++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt index 8212451d5d..879f094179 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ -305,9 +305,10 @@ function validate1(date) {
+ - [% IF ( picture ) %]Delete[% END %] + [% IF ( picture ) %]Delete[% END %]
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt index 28fce72aad..958f810a7c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt @@ -143,6 +143,7 @@
+ Cancel diff --git a/members/moremember.pl b/members/moremember.pl index 9f3ae74bdb..00e26006ce 100755 --- a/members/moremember.pl +++ b/members/moremember.pl @@ -36,6 +36,7 @@ use strict; #use warnings; FIXME - Bug 2505 use CGI qw ( -utf8 ); +use Digest::MD5 qw(md5_base64); use C4::Context; use C4::Auth; use C4::Output; @@ -62,6 +63,7 @@ use DateTime; use Koha::DateUtils; use Koha::Database; use Koha::Patron::Categories; +use Koha::Token; use vars qw($debug); @@ -268,6 +270,14 @@ if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preferen # patronimage related interface on 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 => C4::Context->userenv->{id}, + secret => md5_base64( C4::Context->config('pass') ), + }), +); + $template->param(%$data); diff --git a/tools/picture-upload.pl b/tools/picture-upload.pl index 9892eb5d17..6aeed8ace3 100755 --- a/tools/picture-upload.pl +++ b/tools/picture-upload.pl @@ -25,6 +25,7 @@ use File::Temp; use File::Copy; use CGI qw ( -utf8 ); use GD; +use Digest::MD5 qw(md5_base64); use C4::Context; use C4::Auth; use C4::Output; @@ -34,6 +35,7 @@ use C4::Debug; use Koha::Patrons; use Koha::Patron::Image; use Koha::Patron::Images; +use Koha::Token; my $input = new CGI; @@ -82,6 +84,14 @@ our %errors = (); # Case is important in these operational values as the template must use case to be visually pleasing! if ( ( $op eq 'Upload' ) && $uploadfile ) { + + die "Wrong CSRF token" + unless Koha::Token->new->check_csrf({ + id => C4::Context->userenv->{id}, + secret => md5_base64( C4::Context->config('pass') ), + token => scalar $input->param('csrf_token'), + }); + my $dirname = File::Temp::tempdir( CLEANUP => 1 ); $debug and warn "dirname = $dirname"; my $filesuffix; @@ -175,6 +185,12 @@ if ( $borrowernumber && !%errors && !$template->param('ERRORS') ) { "/cgi-bin/koha/members/moremember.pl?borrowernumber=$borrowernumber"); } else { + $template->param( + csrf_token => Koha::Token->new->generate_csrf({ + id => C4::Context->userenv->{id}, + secret => md5_base64( C4::Context->config('pass') ), + }), + ); output_html_with_http_headers $input, $cookie, $template->output; } -- 2.39.5