C4::Auth_with_case has a $query->param($_) in a context which is not clearly scalar to the interpreter, so it throws a security warning. Introduce a variable, use the variable instead of calling param() twice. Noise disappears.
Created attachment 38763 [details] [review] Bug 14121: Silence warnings t/db_dependent/Auth_with_cas.t TEST PLAN --------- 1) $ prove t/db_dependent/Auth_with_cas.t -- CGI security warning 2) apply patch 3) $ prove t/db_dependent/Auth_with_cas.t -- no noise. 4) koha qa test tools
No warns on current master...
(In reply to Bernardo Gonzalez Kriegel from comment #2) > No warns on current master... Apache 2.4 issue.
(In reply to Bernardo Gonzalez Kriegel from comment #2) > No warns on current master... this is what I have (gitified install): l2c2technologies@gadfly:~/koha$ apache2 -v Server version: Apache/2.4.7 (Ubuntu) Server built: Mar 10 2015 13:05:59 test result l2c2technologies@gadfly:~/koha$ prove t/db_dependent/Auth_with_cas.t t/db_dependent/Auth_with_cas.t .. ok All tests successful. Files=1, Tests=4, 1 wallclock secs ( 0.01 usr 0.01 sys + 0.18 cusr 0.00 csys = 0.20 CPU) Result: PASS
(In reply to M. Tompsett from comment #3) > (In reply to Bernardo Gonzalez Kriegel from comment #2) > > No warns on current master... > > Apache 2.4 issue. Server version: Apache/2.4.7 (Ubuntu) HEAD: 607b224 Bug 13591: Pass OPACBaseURL to rss creation process prove t/db_dependent/Auth_with_cas.t t/db_dependent/Auth_with_cas.t .. ok All tests successful. Files=1, Tests=4, 1 wallclock secs ( 0.04 usr 0.00 sys + 0.29 cusr 0.01 csys = 0.34 CPU) Result: PASS No noise for me!
mtompset@debian:~/kohaclone$ git reset --hard origin/master HEAD is now at 607b224 Bug 13591: Pass OPACBaseURL to rss creation process mtompset@debian:~/kohaclone$ prove t/db_dependent/Auth_with_cas.t t/db_dependent/Auth_with_cas.t .. 1/4 CGI::param called in list context from package C4::Auth_with_cas line 217, 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. CGI::param called in list context from package C4::Auth_with_cas line 217, 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. t/db_dependent/Auth_with_cas.t .. ok All tests successful. Files=1, Tests=4, 1 wallclock secs ( 0.02 usr 0.00 sys + 0.16 cusr 0.01 csys = 0.19 CPU) Result: PASS mtompset@debian:~/kohaclone$ git bz apply 14121 Bug 14121 - Silence warnings t/db_dependent/Auth_with_cas.t 38763 - Bug 14121: Silence warnings t/db_dependent/Auth_with_cas.t Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 14121: Silence warnings t/db_dependent/Auth_with_cas.t mtompset@debian:~/kohaclone$ prove t/db_dependent/Auth_with_cas.t t/db_dependent/Auth_with_cas.t .. ok All tests successful. Files=1, Tests=4, 0 wallclock secs ( 0.01 usr 0.00 sys + 0.16 cusr 0.00 csys = 0.17 CPU) Result: PASS mtompset@debian:~/dhmake$ cpan2deb CGI == dh-make-perl 0.84 == CGI is a standard module. Will not build without --core-ok. mtompset@debian:~/dhmake$ perl -v This is perl 5, version 20, subversion 2 (v5.20.2) built for x86_64-linux-gnu-thread-multi (with 40 registered patches, see perl -V for more detail) Copyright 1987-2015, Larry Wall Perl may be copied only under the terms of either the Artistic License or the GNU General Public License, which may be found in the Perl 5 source kit. Complete documentation for Perl, including FAQ lists, should be found on this system using "man perl" or "perldoc perl". If you have access to the Internet, point your browser at http://www.perl.org/, the Perl Home Page. I'm testing Debian Jessie with perl 5.20.2 Both of you were testing Ubuntu.
I attempted to test with Ubuntu 14.04 LTS which has Perl 5.18.2, and I did not get noise either. This is clearly a newer perl (hence newer CGI) issue.
t/db_dependent/Auth_with_cas.t .. 1/4 # Failed test 'Intranet URL should be returned when using intranet login (Bug 13507)' # at t/db_dependent/Auth_with_cas.t line 60. # got: 'localhost/cgi-bin/koha/opac-user.pl?bar=baz' # expected: 'localhost:8080?bar=baz' # Looks like you failed 1 test of 4. t/db_dependent/Auth_with_cas.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/4 subtests Test Summary Report ------------------- t/db_dependent/Auth_with_cas.t (Wstat: 256 Tests: 4 Failed: 1) Failed test: 4 Non-zero exit status: 1 Files=1, Tests=4, 1 wallclock secs ( 0.02 usr 0.00 sys + 0.23 cusr 0.02 csys = 0.27 CPU) Result: FAIL Setting to Failed QA.
(In reply to Aleisha Amohia from comment #8) > t/db_dependent/Auth_with_cas.t .. 1/4 > # Failed test 'Intranet URL should be returned when using intranet login > (Bug 13507)' > # at t/db_dependent/Auth_with_cas.t line 60. > # got: 'localhost/cgi-bin/koha/opac-user.pl?bar=baz' > # expected: 'localhost:8080?bar=baz' > # Looks like you failed 1 test of 4. > t/db_dependent/Auth_with_cas.t .. Dubious, test returned 1 (wstat 256, 0x100) > Failed 1/4 subtests I can't replicate this failure. If I can't replicate, I can't fix. Could you retest on a fresh branch after a git pull? Thanks.
Created attachment 39621 [details] [review] Bug 14121: Silence warnings t/db_dependent/Auth_with_cas.t TEST PLAN --------- 1) $ prove t/db_dependent/Auth_with_cas.t -- CGI security warning 2) apply patch 3) $ prove t/db_dependent/Auth_with_cas.t -- no noise. 4) koha qa test tools
(In reply to M. Tompsett from comment #7) > I attempted to test with Ubuntu 14.04 LTS which has Perl 5.18.2, and I did > not get noise either. This is clearly a newer perl (hence newer CGI) issue. I tested this on Debian Wheezy which has Perl 5.14.2 with no noise either, so this should be tested on a machine with Debian Jessie, so the issue will be triggered. Test plan in comment #10.
Created attachment 40696 [details] [review] Bug 14121: Silence warnings t/db_dependent/Auth_with_cas.t TEST PLAN --------- 1) $ prove t/db_dependent/Auth_with_cas.t -- CGI security warning 2) apply patch 3) $ prove t/db_dependent/Auth_with_cas.t -- no noise. 4) koha qa test tools Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Cleared a small conflict
(In reply to Nick Clemens from comment #13) > Cleared a small conflict Thanks. Looks like a correct fix. :)
% more /etc/debian_version jessie/sid % perl --version | grep version This is perl 5, version 20, subversion 2 (v5.20.2) built for i586-linux-gnu-thread-multi-64int % git reset --hard origin/master % prove t/db_dependent/Auth_with_cas.t t/db_dependent/Auth_with_cas.t .. ok All tests successful. Files=1, Tests=4, 1 wallclock secs ( 0.02 usr 0.00 sys + 0.96 cusr 0.02 csys = 1.00 CPU) Result: PASS No warn here.
% pmvers CGI 3.65
I suppose it comes from this line: 217 $uri_params_part .= URI::Escape::uri_escape( $query->param($_) ); Could you confirm?
(In reply to Jonathan Druart from comment #17) > I suppose it comes from this line: > 217 $uri_params_part .= URI::Escape::uri_escape( > $query->param($_) ); > > Could you confirm? Yes, the other change is an optimization. It's faster to access a variable than function call twice.
mtompset@debian:~/kohaclone$ cat /etc/debian_version 8.1 mtompset@debian:~/kohaclone$ perl --version | grep version This is perl 5, version 20, subversion 2 (v5.20.2) built for x86_64-linux-gnu-thread-multi mtompset@debian:~/kohaclone$ git reset --hard origin/master HEAD is now at 6e9086f Bug 3206: (QA followup) missing comma on sysprefs.sql mtompset@debian:~/kohaclone$ prove t/db_dependent/Auth_with_cas.t t/db_dependent/Auth_with_cas.t .. 1/4 CGI::param called in list context from package C4::Auth_with_cas line 217, 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. CGI::param called in list context from package C4::Auth_with_cas line 217, 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. t/db_dependent/Auth_with_cas.t .. ok All tests successful. Files=1, Tests=4, 0 wallclock secs ( 0.02 usr 0.00 sys + 0.67 cusr 0.01 csys = 0.70 CPU) Result: PASS mtompset@debian:~/kohaclone$ pmvers CGI 4.09
(In reply to M. Tompsett from comment #19) > mtompset@debian:~/kohaclone$ pmvers CGI > 4.09 Did you install it using from CPAN?
(In reply to Jonathan Druart from comment #20) > Did you install [CGI] using from CPAN? https://packages.debian.org/jessie/libcgi-pm-perl Not that I can tell. I avoid CPAN like the plague. And the latest is 4.21 as far as I can tell. JD wrote: > % more /etc/debian_version > jessie/sid I wrote: > mtompset@debian:~/kohaclone$ cat /etc/debian_version > 8.1 Your debian version is an experimental pre-release of Jessie? Try a fresher install?
Created attachment 40936 [details] [review] [PASSED QA] Bug 14121: Silence warnings t/db_dependent/Auth_with_cas.t TEST PLAN --------- 1) $ prove t/db_dependent/Auth_with_cas.t -- CGI security warning 2) apply patch 3) $ prove t/db_dependent/Auth_with_cas.t -- no noise. 4) koha qa test tools Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Not able to reproduce the error on my setup, but the code is a clear improvement over the previous version.
Patch pushed to master. Thanks Mark!
Pushed to 3.20.x will be in 3.20.2