Skip Menu |

This queue is for tickets about the HTML-Mason CPAN distribution.

Report information
The Basics
Id: 99520
Status: resolved
Priority: 0/
Queue: HTML-Mason

People
Owner: Nobody in particular
Requestors: cpan [...] jibsheet.com
Cc: gregoa [...] cpan.org
AdminCc:

Bug Information
Severity: Important
Broken in: 1.54
Fixed in:
  • 1.55
  • 1.56



Subject: CGI.pm warnings about param() on newer CGI.pms
In response to the bugzilla bugs a few weeks back, CGI.pm in 4.05 and later started warning if you call param() in list context. https://metacpan.org/changes/distribution/CGI http://blog.gerv.net/2014/10/new-class-of-vulnerability-in-perl-web-applications/ This is the relevant warning CGI::param called in list context from package HTML::Mason::Utils line 48, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" Relevant code my @values = map { $q->$_($key) } @methods; $args{$key} = @values == 1 ? $values[0] : \@values; I don't see how this is vulnerable, which means HTML::Mason needs to turn on the "stop that" flag for CGI.pm https://metacpan.org/pod/CGI#Fetching-the-value-or-values-of-a-single-named-parameter I've attached a trivial test that passes RT's test suite locally and quiets warnings in normal usage. It also passes dzil test for HTML-Mason, except for the author-live tests which I'm not set up to run. This came up via Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=765477 -kevin
Subject: 0001-Ask-CGI-to-not-warn-about-param-in-list-context.patch
From 0891bd1f51df3a76b083df9052092837c4f28fe7 Mon Sep 17 00:00:00 2001 From: Kevin Falcone <falcone@bestpractical.com> Date: Wed, 15 Oct 2014 15:28:09 -0400 Subject: [PATCH] Ask CGI to not warn about param() in list context. Mason protects from the class of param() bugs which allow users to sneak in arguments as described here: http://blog.gerv.net/2014/10/new-class-of-vulnerability-in-perl-web-applications/ Since CGI.pm 4.05 the only way to quiet this warning is by setting their variable as documented here https://metacpan.org/pod/CGI#Fetching-the-value-or-values-of-a-single-named-parameter Mason has always allowed foo=1&foo=2 to end up available in a template as @foo = (1,2) so retain backcompat. --- lib/HTML/Mason/Utils.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/HTML/Mason/Utils.pm b/lib/HTML/Mason/Utils.pm index ef9c5b8..c3e814e 100644 --- a/lib/HTML/Mason/Utils.pm +++ b/lib/HTML/Mason/Utils.pm @@ -42,6 +42,7 @@ sub cgi_request_args foreach my $key ( map { $q->$_() } @methods ) { next if exists $args{$key}; + local $CGI::LIST_CONTEXT_WARN = 0; my @values = map { $q->$_($key) } @methods; $args{$key} = @values == 1 ? $values[0] : \@values; } -- 1.9.3
On 2014-10-15 12:42:41, FALCONE wrote: Show quoted text
> In response to the bugzilla bugs a few weeks back, CGI.pm in 4.05 and > later started warning if you call param() in list context. > > https://metacpan.org/changes/distribution/CGI > http://blog.gerv.net/2014/10/new-class-of-vulnerability-in-perl-web- > applications/ > > This is the relevant warning > > CGI::param called in list context from package HTML::Mason::Utils line > 48, this can lead to vulnerabilities. See the warning in "Fetching the > value or values of a single named parameter" > > Relevant code > > my @values = map { $q->$_($key) } @methods; > $args{$key} = @values == 1 ? $values[0] : \@values; > > I don't see how this is vulnerable, which means HTML::Mason needs to > turn on the "stop that" flag for CGI.pm
How about HTML::Mason stop calling param() entirely? This is an ancient interface and there are much better alternatives now.
Subject: Re: [rt.cpan.org #99520] CGI.pm warnings about param() on newer CGI.pms
Date: Wed, 15 Oct 2014 16:40:20 -0400
To: Karen Etheridge via RT <bug-HTML-Mason [...] rt.cpan.org>
From: Kevin Falcone <cpan [...] jibsheet.com>
On Wed, Oct 15, 2014 at 04:13:12PM -0400, Karen Etheridge via RT wrote: Show quoted text
> How about HTML::Mason stop calling param() entirely? This is an > ancient interface and there are much better alternatives now.
HTML::Mason is legacy and very concious of back-compat (Mason is the v2 replacement). I doubt they want to change code that's been there since at least 2001 or switch away from CGI.pm. Suggesting specific alternatives might be helpful, vague assertions are not. -kevin
From: smcv
On Wed Oct 15 16:40:40 2014, FALCONE wrote: Show quoted text
> Suggesting specific alternatives > might be helpful, vague assertions are not.
Here's what we did in ikiwiki, for what it's worth: http://source.ikiwiki.branchable.com/?p=source.git;a=commitdiff;h=cfbcbda0ad848334640ad849ed618873ecba8eb4 (Amitai researched compatibility and found that param_fetch was present in a pre-2000 version of CGI.pm.)
With Kevin's patch, there's still a warning left when running HTML::Mason's test suite: t/13-errors.t ............................. ok CGI::param called in list context from package HTML::Mason::Commands line 1, 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/14-cgi.t ................................ ok Cheers, gregor
Subject: Re: [rt.cpan.org #99520] CGI.pm warnings about param() on newer CGI.pms
Date: Tue, 21 Oct 2014 17:01:25 -0400
To: gregor herrmann via RT <bug-HTML-Mason [...] rt.cpan.org>
From: Kevin Falcone <cpan [...] jibsheet.com>
On Sun, Oct 19, 2014 at 01:44:43PM -0400, gregor herrmann via RT wrote: Show quoted text
> With Kevin's patch, there's still a warning left when running HTML::Mason's test suite: > CGI::param called in list context from package HTML::Mason::Commands line 1, 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/14-cgi.t ................................ ok
Comes from t/14-cgi.t calling param on their mocked up CGI.pm component => q{some <% $m->cgi_object->param('arg') %> cryin'}, So, whatever fix is decided on (the local $CGI::STOPWARNING version or rewriting param fetching) would go in the tests. -kevin
On Tue Oct 21 17:01:53 2014, FALCONE wrote: Show quoted text
> On Sun, Oct 19, 2014 at 01:44:43PM -0400, gregor herrmann via RT > wrote:
> > With Kevin's patch, there's still a warning left when running > > HTML::Mason's test suite: > > CGI::param called in list context from package HTML::Mason::Commands > > line 1, 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/14-cgi.t ................................ ok
> > Comes from t/14-cgi.t calling param on their mocked up CGI.pm > > component => q{some <% $m->cgi_object->param('arg') %> cryin'}, > > So, whatever fix is decided on (the local $CGI::STOPWARNING version or > rewriting param fetching) would go in the tests.
The local solution seems like the simplest and least likely-to-break-anything way to go.
RT-Send-CC: swartz [...] pobox.com
Jon, unless you'd like to do releases of this, I'd ask folks to submit a PR against https://github.com/autarch/HTML-Mason
Subject: Re: [rt.cpan.org #99520] CGI.pm warnings about param() on newer CGI.pms
Date: Tue, 11 Nov 2014 10:01:37 -0500
To: Dave Rolsky via RT <bug-HTML-Mason [...] rt.cpan.org>
From: Kevin Falcone <falcone [...] bestpractical.com>
On Sat, Nov 08, 2014 at 11:19:29PM -0500, Dave Rolsky via RT wrote: Show quoted text
> > So, whatever fix is decided on (the local $CGI::STOPWARNING version or > > rewriting param fetching) would go in the tests.
> > The local solution seems like the simplest and least likely-to-break-anything way to go.
With HTML::Mason being 'biologically stable' that was certainly my intent with the first patch on this ticket. On Sun, Nov 09, 2014 at 11:41:34AM -0500, Dave Rolsky via RT wrote: Show quoted text
> Jon, unless you'd like to do releases of this, I'd ask folks to submit > a PR against https://github.com/autarch/HTML-Mason
I'll turn my original patch and one for the tests into a PR now that I'm back from travel. -kevin
Show quoted text
> I'll turn my original patch and one for the tests into a PR now that > I'm back from travel.
Filed https://github.com/autarch/HTML-Mason/pull/1 -kevin