From 3098a95c4eb8574f2edfb08e8a9840af714eb068 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 13 May 2024 14:47:28 +0200 Subject: [PATCH] Bug 36520: Prevent SQL injection in GetPreparedLetter Actually in _get_tt_params The following query will delay the response SELECT `me`.`biblionumber`, `me`.`frameworkcode`, `me`.`author`, `me`.`title`, `me`.`medium`, `me`.`subtitle`, `me`.`part_number`, `me`.`part_name`, `me`.`unititle`, `me`.`notes`, `me`.`serial`, `me`.`seriestitle` , `me`.`copyrightdate`, `me`.`timestamp`, `me`.`datecreated`, `me`.`abstract` FROM `biblio` `me` WHERE `biblionumber` = '1) AND (SELECT 1 FROM (SELECT(SLEEP(6)))x)-- -' ORDER BY field( biblionumber, 1 ) AND ( SELECT 1 FROM SELECT SLEEP( 6 ) x ) -- - ) To test 1/ Add some items to your cart in the opac 2/ Choose send cart 3/ Open firefox developer tools and switch to the network tab 4/ Send cart 5/ In the network tab, find the post request and choose copy as curl 6/ Edit the curl command to add )+AND+(SELECT+1+FROM+(SELECT(SLEEP(6)))x)--+- to the bib_list parameter 7/ Run the curl notice it takes a long time to respond, if you want to check run the curl without the above part added 8/ Apply the patch and restart plack 9/ Run the modified curl and notice no longer the slow down 10/ Test in browser and make sure the basket is still sent Signed-off-by: Chris Cormack Signed-off-by: Victor Grousset/tuxayo Signed-off-by: Marcel de Rooy (cherry picked from commit 0b3c98b0ba01ea5c886ecfe8eef174b5b7c6ec25) Signed-off-by: Fridolin Somers --- C4/Letters.pm | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index dfb7b78694..c67a878c43 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1890,6 +1890,7 @@ sub _get_tt_params { }, }; + my $dbh = C4::Context->dbh; foreach my $table ( keys %$tables ) { next unless $config->{$table}; @@ -1910,12 +1911,19 @@ sub _get_tt_params { my $objects = $module->search( { $key => $values }, { - # We want to retrieve the data in the same order - # FIXME MySQLism - # field is a MySQLism, but they are no other way to do it - # To be generic we could do it in perl, but we will need to fetch - # all the data then order them - @$values ? ( order_by => \[ "field($key, " . join( ', ', @$values ) . ")" ] ) : () + # We want to retrieve the data in the same order + # FIXME MySQLism + # field is a MySQLism, but they are no other way to do it + # To be generic we could do it in perl, but we will need to fetch + # all the data then order them + @$values + ? ( + order_by => \[ + sprintf "field(%s, %s)", $key, + join(',', map { $dbh->quote($_) } @$values ) + ] + ) + : () } ); $params->{ $config->{$table}->{plural} } = $objects; -- 2.39.5