Skip Menu |
 

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

Report information
The Basics
Id: 23592
Status: resolved
Priority: 0/
Queue: HTML-Template

People
Owner: Nobody in particular
Requestors: sven-bitcard [...] sven.de
Cc:
AdminCc:

Bug Information
Severity: Wishlist
Broken in: (no value)
Fixed in: (no value)



Subject: new option force_untaint
Download (untitled) / with headers
text/plain 203b
This patch to HTML::Template 2.8 (attached) adds a new option "force_untaint". If the option is set, you can no longer pass untainted values to paramters with param(). perl has to run in taint mode (-T).
Subject: force_untaint.patch
diff -dur --unidirectional-new-file HTML-Template-2.8.orig/Changes HTML-Template-2.8/Changes --- HTML-Template-2.8.orig/Changes 2005-12-22 00:37:49.000000000 +0100 +++ HTML-Template-2.8/Changes 2006-11-24 14:55:08.000000000 +0100 @@ -1,3 +1,6 @@ + - New Feature: the new force_untaint option makes sure you do not + pass tainted values to param(). [Sven Neuhaus] + 2.8 Wed Dec 21 18:37:39 EST 2005 - New Feature: the new default_escape option allows you to apply escaping to all variables in a template. [Alex Kapranoff] diff -dur --unidirectional-new-file HTML-Template-2.8.orig/MANIFEST HTML-Template-2.8/MANIFEST --- HTML-Template-2.8.orig/MANIFEST 2005-12-22 00:34:42.000000000 +0100 +++ HTML-Template-2.8/MANIFEST 2006-11-24 15:05:07.000000000 +0100 @@ -11,6 +11,8 @@ t/01coderefs.t t/02random.t t/03else_else_bug.t +t/04no_taintmode.t +t/05force_untaint.t t/99-old-test-pl.t Template.pm templates/case_loop.tmpl diff -dur --unidirectional-new-file HTML-Template-2.8.orig/Makefile.PL HTML-Template-2.8/Makefile.PL --- HTML-Template-2.8.orig/Makefile.PL 2004-08-05 19:50:51.000000000 +0200 +++ HTML-Template-2.8/Makefile.PL 2006-11-24 14:54:12.000000000 +0100 @@ -29,5 +29,6 @@ 'File::Spec' => 0.82, 'Digest::MD5'=> 0, 'Test::More' => 0, + 'Scalar::Util' => 0, }, ); diff -dur --unidirectional-new-file HTML-Template-2.8.orig/Template.pm HTML-Template-2.8/Template.pm --- HTML-Template-2.8.orig/Template.pm 2005-12-22 00:21:01.000000000 +0100 +++ HTML-Template-2.8/Template.pm 2006-11-24 15:36:50.000000000 +0100 @@ -475,6 +475,13 @@ =item * +force_untaint - if set to 1 the module will not allow you +to set parameters with tainted values. This option makes sure you untaint +everything so you don't accidentally introduce e.g. cross-site-scripting +(CSS) vulnerabilities. Requires taint mode. Defaults to 0. + +=item * + strict - if set to 0 the module will allow things that look like they might be TMPL_* tags to get by without dieing. Example: @@ -898,6 +905,7 @@ use Carp; # generate better errors with more context use File::Spec; # generate paths that work on all platforms use Digest::MD5 qw(md5_hex); # generate cache keys +use Scalar::Util qw(tainted); # define accessor constants used to improve readability of array # accesses into "objects". I used to use 'use constant' but that @@ -942,6 +950,7 @@ file_cache => 0, file_cache_dir => '', file_cache_dir_mode => 0700, + force_untaint => 0, cache_debug => 0, shared_cache_debug => 0, memory_debug => 0, @@ -1005,6 +1014,11 @@ delete $options->{source}; } + # make sure taint mode is on if force_untaint flag is set + if ($options->{force_untaint} && ! ${^TAINT}) { + croak("HTML::Template->new() : 'force_untaint' option set but perl does not run in taint mode!"); + } + # associate should be an array of one element if it's not # already an array. if (ref($options->{associate}) ne 'ARRAY') { @@ -2463,6 +2477,9 @@ } ); +An error occurs if you try to set a value that is tainted if the "force_untaint" +option is set. + =cut @@ -2531,6 +2548,9 @@ } else { (ref($param_map->{$param}) eq 'HTML::Template::VAR') or croak("HTML::Template::param() : attempt to set parameter '$param' with a scalar - parameter is not a TMPL_VAR!"); + if ($options->{force_untaint} && tainted($value)) { + croak("HTML::Template::param() : attempt to set parameter '$param' with a tainted value") + } ${$param_map->{$param}} = $value; } } diff -dur --unidirectional-new-file HTML-Template-2.8.orig/t/04no_taintmode.t HTML-Template-2.8/t/04no_taintmode.t --- HTML-Template-2.8.orig/t/04no_taintmode.t 1970-01-01 01:00:00.000000000 +0100 +++ HTML-Template-2.8/t/04no_taintmode.t 2006-11-24 15:02:13.000000000 +0100 @@ -0,0 +1,16 @@ + +use Test::More tests => 1; +use HTML::Template; + +my $text = qq{foo}; + +eval { + my $template = HTML::Template->new( + debug => 0, + scalarref => \$text, + force_untaint => 1, + ); +}; + +like($@, qr/script does not run in taint mode/, + "force_untaint does not work without taint mode"); diff -dur --unidirectional-new-file HTML-Template-2.8.orig/t/05force_untaint.t HTML-Template-2.8/t/05force_untaint.t --- HTML-Template-2.8.orig/t/05force_untaint.t 1970-01-01 01:00:00.000000000 +0100 +++ HTML-Template-2.8/t/05force_untaint.t 2006-11-24 15:03:38.000000000 +0100 @@ -0,0 +1,22 @@ +#!perl -T + +use Test::More tests => 1; +use HTML::Template; + +my $text = qq{ <TMPL_VAR NAME="a"> }; + +my $template = HTML::Template->new( + debug => 0, + scalarref => \$text, + force_untaint => 1, +); + +# We can't manually taint a variable, can we? +# OK, let's use ENV{PATH} - it is usually tainted [sn] + +eval { + $template->param(a => $ENV{PATH} ); +}; + +like($@, qr/attempt to set parameter 'a' with a tainted value/, + "set tained value despite option force_untaint");
From: sven-bitcard [...] sven.de
Download (untitled) / with headers
text/plain 260b
Here is a new version of the patch. force_untaint now has 2 different values: 1 means TMPL_VARs with no ESCAPE-Attribute must be untainted 2 means all TMPL_VARs must be untainted This new version makes sure that data returned from coderefs is untainted, too.
Binary files HTML-Template-2.8.orig/.Template.pm.swp and HTML-Template-2.8/.Template.pm.swp differ diff -dur --unidirectional-new-file HTML-Template-2.8.orig/Changes HTML-Template-2.8/Changes --- HTML-Template-2.8.orig/Changes 2005-12-22 00:37:49.000000000 +0100 +++ HTML-Template-2.8/Changes 2006-11-24 14:55:08.000000000 +0100 @@ -1,3 +1,6 @@ + - New Feature: the new force_untaint option makes sure you do not + pass tainted values to param(). [Sven Neuhaus] + 2.8 Wed Dec 21 18:37:39 EST 2005 - New Feature: the new default_escape option allows you to apply escaping to all variables in a template. [Alex Kapranoff] diff -dur --unidirectional-new-file HTML-Template-2.8.orig/MANIFEST HTML-Template-2.8/MANIFEST --- HTML-Template-2.8.orig/MANIFEST 2005-12-22 00:34:42.000000000 +0100 +++ HTML-Template-2.8/MANIFEST 2006-11-24 15:05:07.000000000 +0100 @@ -11,6 +11,8 @@ t/01coderefs.t t/02random.t t/03else_else_bug.t +t/04no_taintmode.t +t/05force_untaint.t t/99-old-test-pl.t Template.pm templates/case_loop.tmpl diff -dur --unidirectional-new-file HTML-Template-2.8.orig/Makefile.PL HTML-Template-2.8/Makefile.PL --- HTML-Template-2.8.orig/Makefile.PL 2004-08-05 19:50:51.000000000 +0200 +++ HTML-Template-2.8/Makefile.PL 2006-11-24 14:54:12.000000000 +0100 @@ -29,5 +29,6 @@ 'File::Spec' => 0.82, 'Digest::MD5'=> 0, 'Test::More' => 0, + 'Scalar::Util' => 0, }, ); diff -dur --unidirectional-new-file HTML-Template-2.8.orig/Template.pm HTML-Template-2.8/Template.pm --- HTML-Template-2.8.orig/Template.pm 2005-12-22 00:21:01.000000000 +0100 +++ HTML-Template-2.8/Template.pm 2006-11-24 16:37:14.000000000 +0100 @@ -475,6 +475,15 @@ =item * +force_untaint - if set to 1 the module will not allow you to set +unescaped parameters with tainted values. If set to 2 you will have +to untaint all parameters, including ones with the escape attribute. +This option makes sure you untaint everything so you don't accidentally +introduce e.g. cross-site-scripting (CSS) vulnerabilities. Requires +taint mode. Defaults to 0. + +=item * + strict - if set to 0 the module will allow things that look like they might be TMPL_* tags to get by without dieing. Example: @@ -898,6 +907,7 @@ use Carp; # generate better errors with more context use File::Spec; # generate paths that work on all platforms use Digest::MD5 qw(md5_hex); # generate cache keys +use Scalar::Util qw(tainted); # define accessor constants used to improve readability of array # accesses into "objects". I used to use 'use constant' but that @@ -942,6 +952,7 @@ file_cache => 0, file_cache_dir => '', file_cache_dir_mode => 0700, + force_untaint => 0, cache_debug => 0, shared_cache_debug => 0, memory_debug => 0, @@ -1005,6 +1016,11 @@ delete $options->{source}; } + # make sure taint mode is on if force_untaint flag is set + if ($options->{force_untaint} && ! ${^TAINT}) { + croak("HTML::Template->new() : 'force_untaint' option set but perl does not run in taint mode!"); + } + # associate should be an array of one element if it's not # already an array. if (ref($options->{associate}) ne 'ARRAY') { @@ -2132,6 +2148,7 @@ die_on_bad_params => $options->{die_on_bad_params}, loop_context_vars => $options->{loop_context_vars}, case_sensitive => $options->{case_sensitive}, + force_untaint => $options->{force_untaint}, ); } elsif ($which eq 'TMPL_IF' or $which eq 'TMPL_UNLESS' ) { @@ -2463,6 +2480,9 @@ } ); +An error occurs if you try to set a value that is tainted if the "force_untaint" +option is set. + =cut @@ -2667,9 +2687,23 @@ if ($type eq 'SCALAR') { $result .= $$line; } elsif ($type eq 'HTML::Template::VAR' and ref($$line) eq 'CODE') { - defined($$line) and $result .= $$line->($self); + if ( defined($$line) ) { + if ($options->{force_untaint}) { + my $tmp = $$line->($self); + croak("HTML::Template->output() : 'force_untaint' option but coderef returns tainted value") + if tainted($tmp); + $result .= $tmp; + } else { + $result .= $$line->($self); + } + } } elsif ($type eq 'HTML::Template::VAR') { - defined($$line) and $result .= $$line; + if (defined $$line) { + if ($options->{force_untaint} && tainted($$line)) { + croak("HTML::Template->output() : tainted value with 'force_untaint' option"); + } + $result .= $$line; + } } elsif ($type eq 'HTML::Template::LOOP') { if (defined($line->[HTML::Template::LOOP::PARAM_SET])) { eval { $result .= $line->output($x, $options->{loop_context_vars}); }; @@ -2734,8 +2768,14 @@ if (defined($$line)) { if (ref($$line) eq 'CODE') { $_ = $$line->($self); + if ($options->{force_untaint} > 1 && tainted($_)) { + croak("HTML::Template->output() : 'force_untaint' option but coderef returns tainted value"); + } } else { $_ = $$line; + if ($options->{force_untaint} > 1 && tainted($_)) { + croak("HTML::Template->output() : tainted value with 'force_untaint' option"); + } } # straight from the CGI.pm bible. @@ -2754,8 +2794,14 @@ if (defined($$line)) { if (ref($$line) eq 'CODE') { $_ = $$line->($self); + if ($options->{force_untaint} > 1 && tainted($_)) { + croak("HTML::Template->output() : 'force_untaint' option but coderef returns tainted value"); + } } else { $_ = $$line; + if ($options->{force_untaint} > 1 && tainted($_)) { + croak("HTML::Template->output() : tainted value with 'force_untaint' option"); + } } s/\\/\\\\/g; s/'/\\'/g; @@ -2770,8 +2816,14 @@ if (defined($$line)) { if (ref($$line) eq 'CODE') { $_ = $$line->($self); + if ($options->{force_untaint} > 1 && tainted($_)) { + croak("HTML::Template->output() : 'force_untaint' option but coderef returns tainted value"); + } } else { $_ = $$line; + if ($options->{force_untaint} > 1 && tainted($_)) { + croak("HTML::Template->output() : tainted value with 'force_untaint' option"); + } } # Build a char->hex map if one isn't already available unless (exists($URLESCAPE_MAP{chr(1)})) { diff -dur --unidirectional-new-file HTML-Template-2.8.orig/t/04no_taintmode.t HTML-Template-2.8/t/04no_taintmode.t --- HTML-Template-2.8.orig/t/04no_taintmode.t 1970-01-01 01:00:00.000000000 +0100 +++ HTML-Template-2.8/t/04no_taintmode.t 2006-11-24 16:07:31.000000000 +0100 @@ -0,0 +1,16 @@ + +use Test::More tests => 1; +use HTML::Template; + +my $text = qq{foo}; + +eval { + my $template = HTML::Template->new( + debug => 0, + scalarref => \$text, + force_untaint => 1, + ); +}; + +like($@, qr/perl does not run in taint mode/, + "force_untaint does not work without taint mode"); diff -dur --unidirectional-new-file HTML-Template-2.8.orig/t/05force_untaint.t HTML-Template-2.8/t/05force_untaint.t --- HTML-Template-2.8.orig/t/05force_untaint.t 1970-01-01 01:00:00.000000000 +0100 +++ HTML-Template-2.8/t/05force_untaint.t 2006-11-24 16:39:55.000000000 +0100 @@ -0,0 +1,37 @@ +#!perl -T + +use Test::More tests => 3; +use HTML::Template; +use Scalar::Util qw(tainted); + +my $text = qq{ <TMPL_VAR NAME="a"> }; + +my $template = HTML::Template->new( + debug => 0, + scalarref => \$text, + force_untaint => 1, +); + +# We can't manually taint a variable, can we? +# OK, let's use ENV{PATH} - it is usually set and tainted [sn] +ok(tainted($ENV{PATH}), "PATH environment variable must be set and tainted for these tests"); + +$template->param(a => $ENV{PATH} ); +eval { + $template->output(); +}; + +like($@, qr/tainted value with 'force_untaint' option/, + "set tainted value despite option force_untaint"); + +sub tainter { # coderef that returns a tainted value + return $ENV{PATH}; +} + +$template->param(a => \&tainter ); +eval { + $template->output(); +}; + +like($@, qr/'force_untaint' option but coderef returns tainted value/, + "coderef returns tainted value despite option force_untaint");
This is committed for v2.9, coming soon. Thanks!


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.