From 8835e542c2cff98a53aec38e786adf7e720d86f4 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Wed, 25 Jul 2018 14:37:59 +0200 Subject: [PATCH] Bug 21115: Add multi_param call and add divider in cache key in svc/report and opac counterpart Resolve things like: CGI::param called in list context from package CGI::Compile::ROOT::usr_share_koha_prodclone_opac_svc_report line 42, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 436. The cache key in both script looks like: opac:report:id:602018 but should for consistency be: opac:report:id:60:2018 Note: The 2018 here is part of the sql_params and should not be concatenated to the report id. Test plan: Do not yet apply this patch. Make a report public, set cache to 300 secs. Check its output with opac/svc/report. Check for the warn in your log. Apply the patch, restart Plack and flush cache. Check opac/svc/report. Modify your report; e.g. add a simple string to the SELECT. Check opac/svc/report. You should still see cached output. Flush the cache. Check opac/svc/report. You should now see the added text. Signed-off-by: Marcel de Rooy Tested also by clearing individual keys with $cache->clear_from_cache. Signed-off-by: Mark Tompsett Signed-off-by: Chris Cormack Signed-off-by: Nick Clemens (cherry picked from commit bfbbe52ff7ff0ec93825684e4728db705b4100d1) Signed-off-by: Martin Renvoize (cherry picked from commit 2ea99b19988a3fed348d574cb06a50c8f6206f34) Signed-off-by: Fridolin Somers --- opac/svc/report | 4 ++-- svc/report | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/opac/svc/report b/opac/svc/report index f83b12e70e..e33b84182f 100755 --- a/opac/svc/report +++ b/opac/svc/report @@ -39,7 +39,7 @@ if (!$report_rec) { die "There is no such report.\n"; } die "Sorry this report is not public\n" unless $report_rec->{public}; -my @sql_params = $query->param('sql_params'); +my @sql_params = $query->multi_param('sql_params'); my $cache = Koha::Caches->get_instance(); my $cache_active = $cache->is_cache_active; @@ -47,7 +47,7 @@ my ($cache_key, $json_text); if ($cache_active) { $cache_key = "opac:report:" - . ( $report_name ? "name:$report_name" : "id:$report_id" ) + . ( $report_name ? "name:$report_name:" : "id:$report_id:" ) . join( '-', @sql_params ); $json_text = $cache->get_from_cache($cache_key); } diff --git a/svc/report b/svc/report index 0188eaadd7..252a6ed0de 100755 --- a/svc/report +++ b/svc/report @@ -36,7 +36,7 @@ my $report_annotation = $query->param('annotated'); my $report_rec = get_saved_report( $report_name ? { 'name' => $report_name } : { 'id' => $report_id } ); if (!$report_rec) { die "There is no such report.\n"; } -my @sql_params = $query->param('sql_params'); +my @sql_params = $query->multi_param('sql_params'); my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { @@ -52,7 +52,7 @@ my $cache = Koha::Caches->get_instance(); my $cache_active = $cache->is_cache_active; my ($cache_key, $json_text); if ($cache_active) { - $cache_key = "intranet:report:".($report_name ? "name:$report_name" : "id:$report_id") + $cache_key = "intranet:report:".($report_name ? "name:$report_name:" : "id:$report_id:") . join( '-', @sql_params ); $json_text = $cache->get_from_cache($cache_key); } -- 2.39.5