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.
Created attachment 77256 [details] [review] 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.
Note for QA: Looks like we should remove the opac and intranet key from the cache when modifying a report. But that is not the topic of this report. So I use this phenomenon in my test plan.
Created attachment 80365 [details] [review] 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>
(In reply to M. Tompsett from comment #3) > Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Thx
Created attachment 80441 [details] [review] 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> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Awesome work all! Pushed to master for 18.11
Pushed to 18.05.x for 18.05.05
Pushed to 17.11.x for 17.11.12