Bug 31001 - "CGI::param called in list context" warning in basket.pl flooding error log
Summary: "CGI::param called in list context" warning in basket.pl flooding error log
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Peter Vashchuk
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 25790
  Show dependency treegraph
 
Reported: 2022-06-21 13:02 UTC by Peter Vashchuk
Modified: 2023-06-08 22:26 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes the cause of a warning message that appears in the system logs when emailing an order to a vendor (Acquisitions > [select a vendor] > [select a basket] > E-mal order). The warning message was "[WARN] CGI::param called in list context from /kohadevbox/koha/acqui/basket.pl line 175, 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 414.".
Version(s) released in:
22.11.00, 22.05.04, 21.11.11


Attachments
Bug 31001: Fix "CGI::param called in list context" warning in basket.pl (1.96 KB, patch)
2022-06-21 13:11 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 31001: Fix "CGI::param called in list context" warning in basket.pl (2.00 KB, patch)
2022-06-26 18:38 UTC, David Nind
Details | Diff | Splinter Review
Bug 31001: Fix "CGI::param called in list context" warning in basket.pl (2.06 KB, patch)
2022-07-06 21:30 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Vashchuk 2022-06-21 13:02:42 UTC

    
Comment 1 Peter Vashchuk 2022-06-21 13:11:00 UTC
Created attachment 136395 [details] [review]
Bug 31001: Fix "CGI::param called in list context" warning in basket.pl

CGI param basketno should be explicitly scalar,
or else error log gets flooded with this warning:

AH01215: CGI::param called in list context from
/home/vagrant/kohaclone/acqui/basket.pl line 175, 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 412.

This patch fixes it by working with it in a scalar context.
The functionality still remains the same but warning doesn't flood
error log.

To reproduce:
1. Head over to the acquisitions page.
2. Pick existing vendor with email contact info or create a new one.
3. Create a new basket or use existing one, and if it doesn't have
any orders, add a new order to it.
4. Use the "E-mail order" button to send order.
5. Check the error log and find the upper mentioned warning.
(Note: if you're going to test this more than once, you might need
to restart your Plack in order for this warning to get added to your
log file again, reasons of that is that the authors of CGI.pm decided
to "warn only once")
6. Apply the patch.
7. Use the "E-mail order" button again, ensure that the same warning
doesn't get added to the log file again.
Comment 2 David Nind 2022-06-26 18:38:06 UTC
Created attachment 136558 [details] [review]
Bug 31001: Fix "CGI::param called in list context" warning in basket.pl

CGI param basketno should be explicitly scalar,
or else error log gets flooded with this warning:

AH01215: CGI::param called in list context from
/home/vagrant/kohaclone/acqui/basket.pl line 175, 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 412.

This patch fixes it by working with it in a scalar context.
The functionality still remains the same but warning doesn't flood
error log.

To reproduce:
1. Head over to the acquisitions page.
2. Pick existing vendor with email contact info or create a new one.
3. Create a new basket or use existing one, and if it doesn't have
any orders, add a new order to it.
4. Use the "E-mail order" button to send order.
5. Check the error log and find the upper mentioned warning.
(Note: if you're going to test this more than once, you might need
to restart your Plack in order for this warning to get added to your
log file again, reasons of that is that the authors of CGI.pm decided
to "warn only once")
6. Apply the patch.
7. Use the "E-mail order" button again, ensure that the same warning
doesn't get added to the log file again.

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2022-06-26 18:51:24 UTC
Testing notes (using koha-testing-docker):

- The warning message appears in this log file: /var/log/koha/kohadev/plack-intranet-error.log

- Note: Lots of other messages will be included in the log if email is not configured.
Comment 4 Tomás Cohen Arazi 2022-07-06 21:30:35 UTC
Created attachment 137249 [details] [review]
Bug 31001: Fix "CGI::param called in list context" warning in basket.pl

CGI param basketno should be explicitly scalar,
or else error log gets flooded with this warning:

AH01215: CGI::param called in list context from
/home/vagrant/kohaclone/acqui/basket.pl line 175, 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 412.

This patch fixes it by working with it in a scalar context.
The functionality still remains the same but warning doesn't flood
error log.

To reproduce:
1. Head over to the acquisitions page.
2. Pick existing vendor with email contact info or create a new one.
3. Create a new basket or use existing one, and if it doesn't have
any orders, add a new order to it.
4. Use the "E-mail order" button to send order.
5. Check the error log and find the upper mentioned warning.
(Note: if you're going to test this more than once, you might need
to restart your Plack in order for this warning to get added to your
log file again, reasons of that is that the authors of CGI.pm decided
to "warn only once")
6. Apply the patch.
7. Use the "E-mail order" button again, ensure that the same warning
doesn't get added to the log file again.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 5 Tomás Cohen Arazi 2022-07-06 21:37:30 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 6 Lucas Gass 2022-08-12 15:49:05 UTC
Backported to 22.05.x for 22.05.04
Comment 7 Arthur Suzuki 2022-08-16 09:55:06 UTC
pushed to 21.11.x for 21.11.11
Comment 8 Victor Grousset/tuxayo 2022-08-19 20:42:57 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document, marking resolved.