From 5b03c19c124cae4312e1d7aa3b8fd979927b606d Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 13 Jan 2017 17:03:41 +0100 Subject: [PATCH] Bug 17901: Fix possible SQL injection in shelf editing It has been reported that /cgi-bin/koha/opac-shelves.pl?op=edit&referer=view&shelfnumber=146&owner=4&shelfname=testX&sortfield=titleaaaaaa\`&category=1 Could lead to SQL injection Actually it explodes because the generated SQL query is not correctly formated. However it would be good to limit the possible values for sortfield. This vulnerability has been reported by MDSec. Signed-off-by: Mirko Tietgen Signed-off-by: Marcel de Rooy Signed-off-by: Kyle M Hall (cherry picked from commit 45cffd874c62c7b090390c5fb3c955c31f524608) Signed-off-by: Julian Maurice --- opac/opac-shelves.pl | 6 ++++-- virtualshelves/shelves.pl | 5 ++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/opac/opac-shelves.pl b/opac/opac-shelves.pl index 6a1a994df0..729e3a0b84 100755 --- a/opac/opac-shelves.pl +++ b/opac/opac-shelves.pl @@ -101,9 +101,11 @@ if ( $op eq 'add_form' ) { $shelf = Koha::Virtualshelves->find($shelfnumber); if ( $shelf ) { $op = $referer; + my $sortfield = $query->param('sortfield'); + $sortfield = 'title' unless grep {/^$sortfield$/}qw( title author copyrightdate itemcallnumber ); if ( $shelf->can_be_managed( $loggedinuser ) ) { $shelf->shelfname( $query->param('shelfname') ); - $shelf->sortfield( $query->param('sortfield') ); + $shelf->sortfield( $sortfield ); $shelf->allow_add( $query->param('allow_add') ); $shelf->allow_delete_own( $query->param('allow_delete_own') ); $shelf->allow_delete_other( $query->param('allow_delete_other') ); @@ -222,6 +224,7 @@ if ( $op eq 'view' ) { if ( $shelf->can_be_viewed( $loggedinuser ) ) { $category = $shelf->category; my $sortfield = $query->param('sortfield') || $shelf->sortfield; # Passed in sorting overrides default sorting + $sortfield = 'title' unless grep {/^$sortfield$/}qw( title author copyrightdate itemcallnumber ); my $direction = $query->param('direction') || 'asc'; $direction = 'asc' if $direction ne 'asc' and $direction ne 'desc'; my ( $page, $rows ); @@ -308,7 +311,6 @@ if ( $op eq 'view' ) { can_delete_shelf => $shelf->can_be_deleted($loggedinuser), can_remove_biblios => $shelf->can_biblios_be_removed($loggedinuser), can_add_biblios => $shelf->can_biblios_be_added($loggedinuser), - sortfield => $sortfield, itemsloop => \@items, sortfield => $sortfield, direction => $direction, diff --git a/virtualshelves/shelves.pl b/virtualshelves/shelves.pl index 4b9adbbed5..76d8ac1400 100755 --- a/virtualshelves/shelves.pl +++ b/virtualshelves/shelves.pl @@ -91,9 +91,11 @@ if ( $op eq 'add_form' ) { if ( $shelf ) { $op = $referer; + my $sortfield = $query->param('sortfield'); + $sortfield = 'title' unless grep {/^$sortfield$/}qw( title author copyrightdate itemcallnumber ); if ( $shelf->can_be_managed( $loggedinuser ) ) { $shelf->shelfname( scalar $query->param('shelfname') ); - $shelf->sortfield( scalar $query->param('sortfield') ); + $shelf->sortfield( $sortfield ); $shelf->allow_add( scalar $query->param('allow_add') ); $shelf->allow_delete_own( scalar $query->param('allow_delete_own') ); $shelf->allow_delete_other( scalar $query->param('allow_delete_other') ); @@ -194,6 +196,7 @@ if ( $op eq 'view' ) { if ( $shelf ) { if ( $shelf->can_be_viewed( $loggedinuser ) ) { my $sortfield = $query->param('sortfield') || $shelf->sortfield || 'title'; # Passed in sorting overrides default sorting + $sortfield = 'title' unless grep {/^$sortfield$/}qw( title author copyrightdate itemcallnumber ); my $direction = $query->param('direction') || 'asc'; $direction = 'asc' if $direction ne 'asc' and $direction ne 'desc'; my ( $rows, $page ); -- 2.39.5