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 <chrisc@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
parent
da03dbd458
commit
11bf7e7bef
|
|
@ -305,9 +305,10 @@ function validate1(date) {
|
|||
</li>
|
||||
</ol>
|
||||
<fieldset class="action">
|
||||
<input type="hidden" name="csrf_token" value="[% csrf_token %]" />
|
||||
<input type="submit" value="Upload" class="submit" />
|
||||
<input name="op" type="hidden" value="Upload" />
|
||||
[% IF ( picture ) %]<a id="delpicture" href="/cgi-bin/koha/tools/picture-upload.pl?op=Delete&borrowernumber=[% borrowernumber %]" class="delete">Delete</a>[% END %]
|
||||
[% IF ( picture ) %]<a id="delpicture" href="/cgi-bin/koha/tools/picture-upload.pl?op=Delete&borrowernumber=[% borrowernumber %]&csrf_token=[% csrf_token %]" class="delete">Delete</a>[% END %]
|
||||
</fieldset>
|
||||
</fieldset>
|
||||
</form>
|
||||
|
|
|
|||
|
|
@ -143,6 +143,7 @@
|
|||
</ol>
|
||||
</fieldset>
|
||||
<fieldset class="action">
|
||||
<input type="hidden" name="csrf_token" value="[% csrf_token %]" />
|
||||
<input type="hidden" name="op" value="Upload" />
|
||||
<input type="submit" value="Upload" class="submit" />
|
||||
<a href="/cgi-bin/koha/tools/tools-home.pl" class="cancel">Cancel</a>
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue