Bug 192677: Add new test to flag failure-to-filter situations in the templates,…

duplicates.cgi

@@ -74,7 +74,7 @@ my $sortby = formvalue("sortby");
 my $changedsince = formvalue("changedsince", 7);
 my $maxrows = formvalue("maxrows", 100);
 my $openonly = formvalue("openonly");
-my $reverse = formvalue("reverse");
+my $reverse = formvalue("reverse") ? 1 : 0;
 my $product = formvalue("product");
 my $sortvisible = formvalue("sortvisible");
 my @buglist = (split(/[:,]/, formvalue("bug_id")));
@@ -159,8 +159,14 @@ if (!tie(%before, 'AnyDBM_File', "data/duplicates/dupes$whenever",
     $dobefore = 1;
 }
 
+my $origmaxrows = $maxrows;
 detaint_natural($maxrows)
-  || ThrowUserError("invalid_maxrows", { maxrows => $maxrows});
+  || ThrowUserError("invalid_maxrows", { maxrows => $origmaxrows});
+
+my $origchangedsince = $changedsince;
+detaint_natural($changedsince)
+  || ThrowUserError("invalid_changedsince", 
+                    { changedsince => $origchangedsince });
 
 my @bugs;
 my @bug_ids; 

t/008filter.t

+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# The contents of this file are subject to the Mozilla Public
+# License Version 1.1 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of
+# the License at http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS
+# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# rights and limitations under the License.
+#
+# The Original Code are the Bugzilla tests.
+#
+# The Initial Developer of the Original Code is Jacob Steenhagen.
+# Portions created by Jacob Steenhagen are
+# Copyright (C) 2001 Jacob Steenhagen. All
+# Rights Reserved.
+#
+# Contributor(s): Gervase Markham <gerv@gerv.net>
+
+#################
+#Bugzilla Test 8#
+#####filter######
+
+# This test scans all our templates for every directive. Having eliminated
+# those which cannot possibly cause XSS problems, it then checks the rest
+# against the safe list stored in the filterexceptions.pl file. 
+
+# Sample exploit code: '>"><script>alert('Oh dear...')</script>
+
+use strict;
+use lib 't';
+
+use vars qw(%safe);
+
+use Support::Templates;
+use File::Spec 0.82;
+use Test::More tests => $Support::Templates::num_actual_files;
+use Cwd;
+
+# Undefine the record separator so we can read in whole files at once
+my $oldrecsep = $/;
+$/ = undef;
+my $topdir = cwd;
+
+foreach my $path (@Support::Templates::include_paths) {
+    $path =~ m|template/([^/]+)/|;
+    my $lang = $1;
+    chdir $topdir; # absolute path
+    my @testitems = Support::Templates::find_actual_files($path);
+    
+    next unless @testitems;
+    
+    # Some people require this, others don't. No-one knows why.
+    chdir $path; # relative path
+    
+    # We load a %safe list of acceptable exceptions.
+    if (!-r "filterexceptions.pl") {
+        ok(0, "$path has templates but no filterexceptions.pl file. --ERROR");
+        next;
+    }
+    else {
+        do "filterexceptions.pl";
+    }
+    
+    # We preprocess the %safe hash of lists into a hash of hashes. This allows
+    # us to flag which members were not found, and report that as a warning, 
+    # thereby keeping the lists clean.
+    foreach my $file (keys %safe) {
+        my $list = $safe{$file};
+        $safe{$file} = {};
+        foreach my $directive (@$list) {
+            $safe{$file}{$directive} = 0;    
+        }
+    }
+
+    foreach my $file (@testitems) {
+        # There are some files we don't check, because there is no need to
+        # filter their contents due to their content-type.
+        if ($file =~ /\.(txt|png)\.tmpl$/) {
+            ok(1, "($lang) $file is filter-safe");
+            next;
+        }
+        
+        # Read the entire file into a string
+        open (FILE, "<$file") || die "Can't open $file: $!\n";    
+        my $slurp = <FILE>;
+        close (FILE);
+
+        my @unfiltered;
+
+        # /g means we execute this loop for every match
+        # /s means we ignore linefeeds in the regexp matches
+        while ($slurp =~ /\[%(.*?)%\]/gs) {
+            my $directive = $1;
+
+            my @lineno = ($` =~ m/\n/gs);
+            my $lineno = scalar(@lineno) + 1;
+
+            # Comments
+            next if $directive =~ /^[+-]?#/;        
+
+            # Remove any leading/trailing + or - and whitespace.
+            $directive =~ s/^[+-]?\s*//;
+            $directive =~ s/\s*[+-]?$//;
+
+            # Directives
+            next if $directive =~ /^(IF|END|UNLESS|FOREACH|PROCESS|INCLUDE|
+                                     BLOCK|USE|ELSE|NEXT|LAST|DEFAULT|FLUSH|
+                                     ELSIF|SET|SWITCH|CASE)/x;
+
+            # Simple assignments
+            next if $directive =~ /^[\w\.\$]+\s+=\s+/;
+
+            # Conditional literals with either sort of quotes 
+            # There must be no $ in the string for it to be a literal
+            next if $directive =~ /^(["'])[^\$]*[^\\]\1/;
+
+            # Special values always used for numbers
+            next if $directive =~ /^[ijkn]$/;
+            next if $directive =~ /^count$/;
+            
+            # Params
+            next if $directive =~ /^Param\(/;
+
+            # Other functions guaranteed to return OK output
+            next if $directive =~ /^(time2str|GetBugLink)\(/;
+
+            # Safe Template Toolkit virtual methods
+            next if $directive =~ /\.(size)$/;
+
+            # Special Template Toolkit loop variable
+            next if $directive =~ /^loop\.(index|count)$/;
+            
+            # Things which are already filtered
+            # Note: If a single directive prints two things, and only one is 
+            # filtered, we may not catch that case.
+            next if $directive =~ /FILTER\ (html|csv|js|url_quote|quoteUrls|
+                                            time|uri|xml)/x;
+
+            # Exclude those on the nofilter list
+            if (defined($safe{$file}{$directive})) {
+                $safe{$file}{$directive}++;
+                next;
+            };
+
+            # This intentionally makes no effort to eliminate duplicates; to do
+            # so would merely make it more likely that the user would not 
+            # escape all instances when attempting to correct an error.
+            push(@unfiltered, "$lineno:$directive");
+        }  
+
+        my $fullpath = File::Spec->catfile($path, $file);
+        
+        if (@unfiltered) {
+            my $uflist = join("\n  ", @unfiltered);
+            ok(0, "($lang) $fullpath has unfiltered directives:\n  $uflist\n--ERROR");
+        }
+        else {
+            # Find any members of the exclusion list which were not found
+            my @notfound;
+            foreach my $directive (keys %{$safe{$file}}) {
+                push(@notfound, $directive) if ($safe{$file}{$directive} == 0);    
+            }
+
+            if (@notfound) {
+                my $nflist = join("\n  ", @notfound);
+                ok(0, "($lang) $fullpath - FEL has extra members:\n  $nflist\n" . 
+                                                                  "--WARNING");
+            }
+            else {
+                # Don't use the full path here - it's too long and unwieldy.
+                ok(1, "($lang) $file is filter-safe");
+            }
+        }
+    }
+}
+
+$/ = $oldrecsep;
+
+exit 0;

template/en/default/account/auth/login.html.tmpl

@@ -34,7 +34,7 @@
   I need a legitimate login and password to continue.
 </p>
 
-<form action="[% target %]" method="POST">
+<form action="[% target FILTER html %]" method="POST">
   <table>
     <tr>
       <td align="right">

template/en/default/attachment/edit.html.tmpl

@@ -51,7 +51,7 @@
       // If this is a plaintext document, remove cruft that Mozilla adds
       // because it treats it as an HTML document with a big PRE section.
       // http://bugzilla.mozilla.org/show_bug.cgi?id=86012
-      var contentType = '[% contenttype %]';
+      var contentType = '[% contenttype FILTER js %]';
       if ( contentType == 'text/plain' )
         {
           theContent = theContent.replace( /^<html><head\/?><body><pre>/i , "" );

template/en/default/bug/create/create-guided.html.tmpl

@@ -211,8 +211,10 @@ function PutDescription() {
 <form method="post" action="post_bug.cgi">  
   <input type="hidden" name="format" value="guided">
   <input type="hidden" name="assigned_to" value="">
-  <input type="hidden" name="priority" value="[% default.priority %]">
-  <input type="hidden" name="version" value="[% default.version %]">
+  <input type="hidden" name="priority" 
+         value="[% default.priority FILTER html %]">
+  <input type="hidden" name="version" 
+         value="[% default.version FILTER html %]">
   
 <table valign="top" cellpadding="5" cellspacing="5" border="0">
 

template/en/default/bug/create/create.html.tmpl

@@ -107,7 +107,8 @@
     [% sel = { description => 'Priority', name => 'priority' } %]
     [% INCLUDE select %]
   [% ELSE %]
-    <input type="hidden" name="priority" value="[% default.priority %]">
+    <input type="hidden" name="priority" 
+           value="[% default.priority FILTER html %]">
   [% END %]
 
   [% sel = { description => 'Severity', name => 'bug_severity' } %]

template/en/default/bug/create/make-template.html.tmpl

@@ -25,7 +25,7 @@
 %]
 
 <p>
-If you bookmark <a href="enter_bug.cgi?[% url %]">this link</a>,
+If you bookmark <a href="enter_bug.cgi?[% url FILTER html %]">this link</a>,
 going to the bookmark will bring up the enter bug page with the fields
 initialized as you've requested.
 </p>

template/en/default/bug/show-multiple.html.tmpl

@@ -106,7 +106,8 @@
     <tr>
       <td colspan="4">
         <b>URL:</b>&nbsp;
-        <a href="[% bug.bug_file_loc %]">[% bug.bug_file_loc FILTER html %]</a> 
+        <a href="[% bug.bug_file_loc FILTER html %]">
+                 [% bug.bug_file_loc FILTER html %]</a> 
     </tr>
     
     <tr>

template/en/default/global/choose-product.html.tmpl

@@ -41,7 +41,7 @@
   <tr>
     <th align="right" valign="top">
       <a href="[% target %]?product=[% p FILTER url_quote %]
-                                       [%- "&amp;format=$format" IF format %]">
+            [% IF format %]&amp;format=[% format FILTER url_quote %][% END %]">
       [% p FILTER html %]</a>:
     </th>
 

template/en/default/global/code-error.html.tmpl

@@ -86,7 +86,7 @@
 
   [% ELSIF error == "field_type_mismatch" %]
     Cannot seem to handle <code>[% field %]</code>
-    and <code>[% type %]</code> together.
+    and <code>[% type FILTER html %]</code> together.
  
   [% ELSIF error == "gd_not_installed" %]
     Charts will not work without the GD Perl module being installed.

template/en/default/global/hidden-fields.html.tmpl

@@ -32,11 +32,11 @@
   [% NEXT IF exclude && field.key.search(exclude) %]
   [% IF mform.${field.key}.size > 1 %]
     [% FOREACH mvalue = mform.${field.key} %]
-      <input type="hidden" name="[% field.key %]"
+      <input type="hidden" name="[% field.key FILTER html %]"
              value="[% mvalue | html | html_linebreak %]">
     [% END %]
   [% ELSE %]
-    <input type="hidden" name="[% field.key %]"
+    <input type="hidden" name="[% field.key FILTER html %]"
            value="[% field.value | html | html_linebreak %]">
   [% END %]
 [% END %]

template/en/default/global/message.html.tmpl

@@ -34,7 +34,7 @@
 [%# Display a URL if the calling script or message block has included one. %]
 [% IF url && link %]
   <p>
-    <a href="[% url %]">[% link %]</a>
+    <a href="[% url FILTER html %]">[% link FILTER html %]</a>
   </p>
 [% END %]
 

template/en/default/global/user-error.html.tmpl

@@ -235,7 +235,7 @@
     
   [% ELSIF error == "illegal_date" %]
     [% title = "Your Query Makes No Sense" %]
-    '<tt>[% date %]</tt>' is not a legal date.
+    '<tt>[% date FILTER html %]</tt>' is not a legal date.
     
   [% ELSIF error == "illegal_email_address" %]
     [% title = "Invalid Email Address" %]
@@ -290,6 +290,11 @@
     in your browser. To help us fix this limitation, add your comments to 
     <a href="http://bugzilla.mozilla.org/show_bug.cgi?id=70907">bug 70907</a>.
 
+  [% ELSIF error == "invalid_changedsince" %]
+    [% title = "Invalid 'Changed Since'" %]
+    The 'changed since' value, '[% changedsince FILTER html %]', must be an
+    integer >= 0.
+
   [% ELSIF error == "invalid_content_type" %]
     [% title = "Invalid Content-Type" %]
     The content type <em>[% contenttype FILTER html %]</em> is invalid.
@@ -355,7 +360,7 @@
   [% ELSIF error == "missing_email_type" %]
     [% title = "Your Query Makes No Sense" %]
     You must specify one or more fields in which to search for
-    <tt>[% email %]</tt>.
+    <tt>[% email FILTER html %]</tt>.
     
   [% ELSIF error == "missing_query" %]
     [% title = "Missing Query" %]

template/en/default/list/change-columns.html.tmpl

@@ -36,7 +36,7 @@
 [% field_descs.qa_contact_realname  = "QA Contact Realname" %]
 
 <form action="colchange.cgi">
-  <input type="hidden" name="rememberedquery" value="[% buffer %]">
+  <input type="hidden" name="rememberedquery" value="[% buffer FILTER html %]">
   [% FOREACH column = masterlist %]
     <input type="checkbox" id="[% column %]" name="column_[% column %]" 
       [% "checked='checked'" IF lsearch(collist, column) != -1 %]>
@@ -65,7 +65,7 @@
 </form>
 
 <form action="colchange.cgi">
-  <input type="hidden" name="rememberedquery" value="[% buffer %]">
+  <input type="hidden" name="rememberedquery" value="[% buffer FILTER html %]">
   <input type="hidden" name="resetit" value="1">
   <input type="submit" value="Reset to Bugzilla default">
 </form>

template/en/default/list/list.html.tmpl

@@ -95,7 +95,7 @@
   <p>
     <a href="query.cgi">Query Page</a>
     &nbsp;&nbsp;<a href="enter_bug.cgi">Enter New Bug</a>
-    <a href="query.cgi?[% urlquerypart %]">Edit this query</a>
+    <a href="query.cgi?[% urlquerypart FILTER html %]">Edit this query</a>
   </p>
 
 [% ELSIF bugs.size == 1 %]
@@ -133,11 +133,13 @@
     <input type="hidden" name="buglist" value="[% buglist %]">
     <input type="submit" value="Long Format">
     &nbsp;&nbsp;
-    <a href="buglist.cgi?[% urlquerypart %]&amp;ctype=csv">CSV</a> &nbsp;&nbsp;
-    <a href="colchange.cgi?[% urlquerypart %]">Change Columns</a> &nbsp;&nbsp;
+    <a href="buglist.cgi?
+             [% urlquerypart FILTER html %]&amp;ctype=csv">CSV</a> &nbsp;&nbsp;
+    <a href="colchange.cgi?
+                [% urlquerypart FILTER html %]">Change Columns</a> &nbsp;&nbsp;
 
     [% IF bugs.size > 1 && caneditbugs && !dotweak %]
-      <a href="buglist.cgi?[% urlquerypart %]
+      <a href="buglist.cgi?[% urlquerypart FILTER html %]
         [%- "&order=$qorder" FILTER html IF order %]&amp;tweak=1">Change Several 
         Bugs at Once</a>
       &nbsp;&nbsp;
@@ -147,7 +149,8 @@
       <a href="mailto:[% bugowners %]">Send Mail to Bug Owners</a> &nbsp;&nbsp;
     [% END %]
 
-    <a href="query.cgi?[% urlquerypart %]">Edit this Query</a> &nbsp;&nbsp;
+    <a href="query.cgi?
+               [% urlquerypart FILTER html %]">Edit this Query</a> &nbsp;&nbsp;
 
   </form>
 

template/en/default/list/table.html.tmpl

@@ -82,7 +82,8 @@
 
     <tr align="left">
       <th colspan="[% splitheader ? 2 : 1 %]">
-        <a href="buglist.cgi?[% urlquerypart %]&amp;order=bugs.bug_id">ID</a>
+        <a href="buglist.cgi?
+                  [% urlquerypart FILTER html %]&amp;order=bugs.bug_id">ID</a>
       </th>
 
       [% IF splitheader %]
@@ -115,7 +116,7 @@
 
 [% BLOCK columnheader %]
   <th colspan="[% splitheader ? 2 : 1 %]">
-    <a href="buglist.cgi?[% urlquerypart %]&amp;order=
+    <a href="buglist.cgi?[% urlquerypart FILTER html %]&amp;order=
       [% column.name FILTER url_quote FILTER html %]
       [% ",$qorder" FILTER html IF order %]">
         [%- abbrev.$id.title || field_descs.$id || column.title -%]</a>

template/en/default/reports/duplicates.html.tmpl

@@ -58,7 +58,7 @@
 <h3><a name="params">Change Parameters</a></h3>
 
 <form method="get" action="duplicates.cgi">
-  <input type="hidden" name="sortby" value="[% sortby %]">
+  <input type="hidden" name="sortby" value="[% sortby FILTER html %]">
   <input type="hidden" name="reverse" value="[% reverse %]">
   <input type="hidden" name="bug_id" value="[% bug_ids_string %]">
   <table>

template/en/default/search/form.html.tmpl

@@ -330,7 +330,7 @@ function selectProduct(f) {
 [% PROCESS "global/field-descs.none.tmpl" %]
 
 [%# If we resubmit to ourselves, we need to know if we are using a format. %]
-<input type="hidden" name="query_format" value="[% format %]">
+<input type="hidden" name="query_format" value="[% format FILTER html %]">
 
 [%# *** Summary *** %]