Attachment 80365 Details for Bug 21115 – Bug 21115: Add multi_param call and add divider in cache key in svc/report and opac counterpart

[patch] Bug 21115: Add multi_param call and add divider in cache key in svc/report and opac counterpart

Bug-21115-Add-multiparam-call-and-add-divider-in-c.patch (text/plain), 3.17 KB, created by Mark Tompsett on 2018-10-10 16:03:27 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Tompsett

Created: 2018-10-10 16:03:27 UTC

Size: 3.17 KB

patch

obsolete

>From bbad56a753788a6560c86cc966394e86c5e8a4ce Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>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 <m.de.rooy@rijksmuseum.nl>
>Tested also by clearing individual keys with $cache->clear_from_cache.
>
>Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
>---
> 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 ddf445fa32..b7c91794e6 100755
>--- a/opac/svc/report
>+++ b/opac/svc/report
>@@ -41,7 +41,7 @@ my $report_rec = $report_recs->next();
> 
> 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;
>@@ -49,7 +49,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 3c2ad96a5c..8718dc4517 100755
>--- a/svc/report
>+++ b/svc/report
>@@ -38,7 +38,7 @@ my $report_recs = Koha::Reports->search( $report_name ? { 'report_name' => $repo
> if (!$report_recs || $report_recs->count == 0 ) { die "There is no such report.\n"; }
> my $report_rec = $report_recs->next();
> 
>-my @sql_params  = $query->param('sql_params');
>+my @sql_params  = $query->multi_param('sql_params');
> 
> my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
>     {
>@@ -54,7 +54,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 ? "report_name:$report_name" : "id:$report_id")
>+    $cache_key = "intranet:report:".($report_name ? "report_name:$report_name:" : "id:$report_id:")
>     . join( '-', @sql_params );
>     $json_text = $cache->get_from_cache($cache_key);
> }
>-- 
>2.17.1

Actions: View | Diff | Splinter Review

Attachments on bug 21115: 77256 | 80365 | 80441