Bug #100503 for Template-Toolkit: New warnings in CGI 4.0x, indicating security issue

Fri Nov 21 14:18:53 2014 SREZIC [...] cpan.org - Ticket created

Subject:

New warnings in CGI 4.0x

When running the Template-Toolkit test suite and a rather new CGI.pm is installed, then the following warnings appear: CGI::param called in list context from package Template::Document line 2, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/perl5.21.6/lib/site_perl/5.21.6/CGI.pm line 436, <DATA> line 1. CGI::param called in list context from package Template::Document line 1, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/perl5.21.6/lib/site_perl/5.21.6/CGI.pm line 436, <DATA> line 1. CGI::param called in list context from package Template::Document line 2, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/perl5.21.6/lib/site_perl/5.21.6/CGI.pm line 436, <DATA> line 1. CGI::param called in list context from package Template::Document line 2, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/perl5.21.6/lib/site_perl/5.21.6/CGI.pm line 436, <DATA> line 1. CGI::param called in list context from package Template::Document line 2, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/perl5.21.6/lib/site_perl/5.21.6/CGI.pm line 436, <DATA> line 1. CGI::param called in list context from package Template::Document line 2, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/perl5.21.6/lib/site_perl/5.21.6/CGI.pm line 436, <DATA> line 1. t/cgi.t ............... ok

Sat Jan 17 17:16:43 2015 ether [...] cpan.org - Subject changed from 'New warnings in CGI 4.0x' to 'New warnings in CGI 4.0x, indicating security issue'

Mon Jan 26 15:20:10 2015 ether [...] cpan.org - Requestor ETHER added

Mon Aug 10 12:28:28 2015 ether [...] cpan.org - Severity Important added

Fri Jan 12 19:43:55 2018 KENTNL [...] cpan.org - Correspondence added

This looks like a side effect of Automagical listification :

https://metacpan.org/pod/distribution/Template-Toolkit/lib/Template/Manual/VMethods.pod#Automagic-Promotion-of-Scalar-to-List-for-Virtual-Methods

https://metacpan.org/pod/distribution/Template-Toolkit/lib/Template/Manual/Variables.pod#Passing-Parameters-and-Returning-Values

"Where code returns a list of multiple values the items will automatically be folded into a list reference which can be accessed as per normal"

Which means by necessity, the following template code in the test executes in list context:

[% global.cgi.param('name') %]

And, to the best of my understanding, there's no way to tell TT to force scalar context on the function from a calling context.

Fixing the test to stop warning is "easy", but gross ( first employed only for defensive clarity, test performs identically without it )

diff --git a/t/cgi.t b/t/cgi.t
index 023ab5ab..f31dfeaa 100644
--- a/t/cgi.t
+++ b/t/cgi.t
@@ -50,27 +50,27 @@ sub barf {
__END__
-- test --
[% USE cgi = CGI('id=abw&name=Andy+Wardley'); global.cgi = cgi -%]
-name: [% global.cgi.param('name') %]
+name: [% global.cgi.multi_param('name').first %]
-- expect --
name: Andy Wardley

-- test --
-name: [% global.cgi.param('name') %]
+name: [% global.cgi.multi_param('name').first %]

-- expect --
name: Andy Wardley

-- test --
-[% FOREACH key = global.cgi.param.sort -%]
-   * [% key %] : [% global.cgi.param(key) %]
+[% FOREACH key = global.cgi.multi_param.sort -%]
+   * [% key %] : [% global.cgi.multi_param(key).first %]
[% END %]
-- expect --
    * id : abw
    * name : Andy Wardley

-- test --
-[% FOREACH key = global.cgi.param().sort -%]
-   * [% key %] : [% global.cgi.param(key) %]
+[% FOREACH key = global.cgi.multi_param().sort -%]
+   * [% key %] : [% global.cgi.multi_param(key).first %]

I'm however not sure if this is the right approach, nor what it means for existing people combining CGI.pm and TT

--
- CPAN kentnl@cpan.org
- Gentoo Perl Maintainer kentnl@gentoo.org ( perl@gentoo.org )

Fri Jan 12 19:43:55 2018 The RT System itself - Status changed from 'new' to 'open'

Fri Jan 12 19:58:25 2018 KENTNL [...] cpan.org - Correspondence added

Here's a slightly less objectionable test patch, that might at very least constitute some kind of good practice for CGI consumers, by using Template::Plugin::Scalar to make it work.

--
- CPAN kentnl@cpan.org
- Gentoo Perl Maintainer kentnl@gentoo.org ( perl@gentoo.org )

Subject:

0001-Fix-tests-warning-w-CGI.pm.patch

From f9b29ce79d9c139daa5f345e4db3c5040f1e5000 Mon Sep 17 00:00:00 2001 From: Kent Fredric <kentnl@gentoo.org> Date: Sat, 13 Jan 2018 13:48:31 +1300 Subject: Fix tests warning w/ CGI.pm This currently seems like an intractable problem with the syntax of Template::Toolkit forcing list context by default on called functions. The only real way around this is to either: A) always use Template::Plugin::Scalar to enforce scalar context B) abuse cgi.multi_param to simply silence the warning and being an adult about the fact "yes, this returns a list, make sure you do the right thing with that" Bug: https://rt.cpan.org/Ticket/Display.html?id=100503 --- t/cgi.t | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/t/cgi.t b/t/cgi.t index 023ab5ab..6086e145 100644 --- a/t/cgi.t +++ b/t/cgi.t @@ -49,28 +49,32 @@ sub barf { __END__ -- test -- +[% USE scalar -%] [% USE cgi = CGI('id=abw&name=Andy+Wardley'); global.cgi = cgi -%] -name: [% global.cgi.param('name') %] +name: [% global.cgi.scalar.param('name') %] -- expect -- name: Andy Wardley -- test -- -name: [% global.cgi.param('name') %] +[% USE scalar -%] +name: [% global.cgi.scalar.param('name') %] -- expect -- name: Andy Wardley -- test -- -[% FOREACH key = global.cgi.param.sort -%] - * [% key %] : [% global.cgi.param(key) %] +[% USE scalar -%] +[% FOREACH key = global.cgi.multi_param.sort -%] + * [% key %] : [% global.cgi.scalar.param(key) %] [% END %] -- expect -- * id : abw * name : Andy Wardley -- test -- -[% FOREACH key = global.cgi.param().sort -%] - * [% key %] : [% global.cgi.param(key) %] +[% USE scalar -%] +[% FOREACH key = global.cgi.multi_param().sort -%] + * [% key %] : [% global.cgi.scalar.param(key) %] [% END %] -- expect -- * id : abw -- 2.15.1

Fri Oct 05 12:52:37 2018 me [...] eboxr.com - Correspondence added

Ticket migrated to github as https://github.com/abw/Template2/issues/168

Fri Oct 05 12:52:37 2018 me [...] eboxr.com - Status changed from 'open' to 'resolved'