From: lpsolit%gmail.com <> Date: Sun, 15 Oct 2006 03:37:48 +0000 (+0000) Subject: Bug 206037: [SECURITY] Fix escaping/quoting in edit*.cgi scripts - Patch by Frédéric... X-Git-Tag: bugzilla-2.18.6~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d52b3be5801d631ab4f9b03509760189200afad6;p=thirdparty%2Fbugzilla.git Bug 206037: [SECURITY] Fix escaping/quoting in edit*.cgi scripts - Patch by Frédéric Buclin r=justdave a=justdave --- diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index ea8d246da5..631176fc8b 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -57,6 +57,8 @@ use base qw(Exporter); GRANT_DIRECT GRANT_DERIVED GRANT_REGEXP + + SAFE_PROTOCOLS ); @Bugzilla::Constants::EXPORT_OK = qw(contenttypes); @@ -191,4 +193,9 @@ use constant GRANT_DIRECT => 0; use constant GRANT_DERIVED => 1; use constant GRANT_REGEXP => 2; +# Protocols which are considered as safe. +use constant SAFE_PROTOCOLS => ('afs', 'cid', 'ftp', 'gopher', 'http', 'https', + 'irc', 'mid', 'news', 'nntp', 'prospero', 'telnet', + 'view-source', 'wais'); + 1; diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index a25e95f1ef..c0e0bd6107 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -319,7 +319,9 @@ sub create { $var =~ s/\@/\@/g; return $var; }, - + + html_light => \&Bugzilla::Util::html_light_quote, + # iCalendar contentline filter ics => [ sub { my ($context, @args) = @_; diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index bbbdc0f2fb..9713bbbcf5 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -31,10 +31,11 @@ use base qw(Exporter); @Bugzilla::Util::EXPORT = qw(is_tainted trick_taint detaint_natural detaint_signed html_quote url_quote value_quote xml_quote - css_class_quote + css_class_quote html_light_quote lsearch max min trim format_time clean_text); +use Bugzilla::Constants; use Bugzilla::Config; # This is from the perlsec page, slightly modifed to remove a warning @@ -80,6 +81,93 @@ sub html_quote { return $var; } +sub html_light_quote { + my ($text) = @_; + + # List of allowed HTML elements having no attributes. + my @allow = qw(b strong em i u p br abbr acronym ins del cite code var + dfn samp kbd big small sub sup tt dd dt dl ul li ol); + + # Are HTML::Scrubber and HTML::Parser installed? + eval { require HTML::Scrubber; + require HTML::Parser; + }; + + # We need utf8_mode() from HTML::Parser 3.40 if running Perl >= 5.8. + if ($@ || ($] >= 5.008 && $HTML::Parser::VERSION < 3.40)) { # Package(s) not installed. + my $safe = join('|', @allow); + my $chr = chr(1); + + # First, escape safe elements. + $text =~ s#<($safe)>#$chr$1$chr#go; + $text =~ s##$chr/$1$chr#go; + # Now filter < and >. + $text =~ s#<#<#g; + $text =~ s#>#>#g; + # Restore safe elements. + $text =~ s#$chr/($safe)$chr##go; + $text =~ s#$chr($safe)$chr#<$1>#go; + return $text; + } + else { # Packages installed. + # We can be less restrictive. We can accept elements with attributes. + push(@allow, qw(a blockquote q span)); + + # Allowed protocols. + my $safe_protocols = join('|', SAFE_PROTOCOLS); + my $protocol_regexp = qr{(^(?:$safe_protocols):|^[^:]+$)}i; + + # Deny all elements and attributes unless explicitly authorized. + my @default = (0 => { + id => 1, + name => 1, + class => 1, + '*' => 0, # Reject all other attributes. + } + ); + + # Specific rules for allowed elements. If no specific rule is set + # for a given element, then the default is used. + my @rules = (a => { + href => $protocol_regexp, + title => 1, + id => 1, + name => 1, + class => 1, + '*' => 0, # Reject all other attributes. + }, + blockquote => { + cite => $protocol_regexp, + id => 1, + name => 1, + class => 1, + '*' => 0, # Reject all other attributes. + }, + 'q' => { + cite => $protocol_regexp, + id => 1, + name => 1, + class => 1, + '*' => 0, # Reject all other attributes. + }, + ); + + my $scrubber = HTML::Scrubber->new(default => \@default, + allow => \@allow, + rules => \@rules, + comment => 0, + process => 0); + + # Avoid filling the web server error log with Perl 5.8.x. + # In HTML::Scrubber 0.08, the HTML::Parser object is stored in + # the "_p" key, but this may change in future versions. + if ($] >= 5.008 && ref($scrubber->{_p}) eq 'HTML::Parser') { + $scrubber->{_p}->utf8_mode(1); + } + return $scrubber->scrub($text); + } +} + # This orignally came from CGI.pm, by Lincoln D. Stein sub url_quote { my ($toencode) = (@_); @@ -294,6 +382,12 @@ be done in the template where possible. Returns a value quoted for use in HTML, with &, E, E, and E<34> being replaced with their appropriate HTML entities. +=item C + +Returns a string where only explicitly allowed HTML elements and attributes +are kept. All HTML elements and attributes not being in the whitelist are either +escaped (if HTML::Scrubber is not installed) or removed. + =item C Quotes characters so that they may be included as part of a url. diff --git a/checksetup.pl b/checksetup.pl index e632c0fac2..ac8e98a9ee 100755 --- a/checksetup.pl +++ b/checksetup.pl @@ -347,6 +347,8 @@ my $xmlparser = have_vers("XML::Parser",0); my $gdgraph = have_vers("GD::Graph",0); my $gdtextalign = have_vers("GD::Text::Align",0); my $patchreader = have_vers("PatchReader","0.9.4"); +my $html_parser = have_vers("HTML::Parser", ($] >= 5.008) ? "3.40" : 0); +my $scrubber = have_vers("HTML::Scrubber", 0); print "\n" unless $silent; @@ -386,6 +388,15 @@ if (!$patchreader && !$silent) { print "install the \nPatchReader module:\n"; print "PatchReader: " . install_command("PatchReader") . "\n"; } +if ((!$scrubber || !$html_parser) && !$silent) { + print "If you want additional HTML tags within product and group "; + print "descriptions,\nyou should install:\n"; + print "HTML::Scrubber: " . install_command("HTML::Scrubber") . "\n" + if !$scrubber; + print "HTML::Parser: " . install_command("HTML::Parser") . "\n" + if !$html_parser; + print "\n"; +} if (%missing) { print "\n\n"; diff --git a/editcomponents.cgi b/editcomponents.cgi index 81d2ee9837..5a1f125af7 100755 --- a/editcomponents.cgi +++ b/editcomponents.cgi @@ -77,7 +77,7 @@ sub CheckProduct ($) } unless (TestProduct $prod) { - print "Sorry, product '$prod' does not exist."; + print "Sorry, product '" . html_quote($prod) . "' does not exist."; PutTrailer(); exit; } @@ -109,7 +109,8 @@ sub CheckComponent ($$) CheckProduct($prod); unless (TestComponent $prod,$comp) { - print "Sorry, component '$comp' for product '$prod' does not exist."; + print "Sorry, component '" . html_quote($comp) . "' for product '" . + html_quote($prod) . "' does not exist."; PutTrailer(); exit; } @@ -255,8 +256,8 @@ unless ($product) { my ($product, $description, $bugs) = FetchSQLData(); $description ||= "missing"; print "\n"; - print " $product\n"; - print " $description\n"; + print " ", html_quote($product), "\n"; + print " " . html_light_quote($description) . "\n"; if ($dobugcounts) { $bugs ||= "none"; print " $bugs\n"; @@ -312,10 +313,10 @@ unless ($action) { my $initialowner = $initialownerid ? DBID_to_name ($initialownerid) : "missing"; my $initialqacontact = $initialqacontactid ? DBID_to_name ($initialqacontactid) : "missing"; print "\n"; - print " $component\n"; - print " $desc\n"; - print " $initialowner\n"; - print " $initialqacontact\n" + print " ", html_quote($component), "\n"; + print " " . html_light_quote($desc) . "\n"; + print " " . html_quote($initialowner) . "\n"; + print " " . html_quote($initialqacontact) . "\n" if Param('useqacontact'); if ($dobugcounts) { $bugs ||= 'none'; @@ -391,7 +392,7 @@ if ($action eq 'new') { exit; } if (TestComponent($product,$component)) { - print "The component '$component' already exists. Please press\n"; + print "The component '" . html_quote($component) . "' already exists. Please press\n"; print "Back and try again.\n"; PutTrailer($localtrailer); exit; @@ -406,7 +407,8 @@ if ($action eq 'new') { my $description = trim($::FORM{description} || ''); if ($description eq '') { - print "You must enter a description for the component '$component'. Please press\n"; + print "You must enter a description for the component '" . + html_quote($component) . "'. Please press\n"; print "Back and try again.\n"; PutTrailer($localtrailer); exit; @@ -415,7 +417,8 @@ if ($action eq 'new') { my $initialowner = trim($::FORM{initialowner} || ''); if ($initialowner eq '') { - print "You must enter an initial owner for the component '$component'. Please press\n"; + print "You must enter an initial owner for the component '" . + html_quote($component) . "'. Please press\n"; print "Back and try again.\n"; PutTrailer($localtrailer); exit; @@ -423,8 +426,8 @@ if ($action eq 'new') { my $initialownerid = DBname_to_id ($initialowner); if (!$initialownerid) { - print "You must use an existing Bugzilla account as initial owner for the component -'$component'. Please press\n"; + print "You must use an existing Bugzilla account as initial owner for the component'" . + html_quote($component) . "'. Please press\n"; print "Back and try again.\n"; PutTrailer($localtrailer); exit; @@ -434,7 +437,8 @@ if ($action eq 'new') { my $initialqacontactid = DBname_to_id ($initialqacontact); if (Param('useqacontact')) { if (!$initialqacontactid && $initialqacontact ne '') { - print "You must use an existing Bugzilla account as initial QA contact for the component '$component'. Please press\n"; + print "You must use an existing Bugzilla account as initial QA contact for the component '" . + html_quote($component) . "'. Please press\n"; print "Back and try again.\n"; PutTrailer($localtrailer); exit; @@ -533,7 +537,8 @@ if ($action eq 'del') { my $initialowner = $initialownerid ? DBID_to_name ($initialownerid) : "missing"; my $initialqacontact = $initialqacontactid ? DBID_to_name ($initialqacontactid) : "missing"; - my $milestonelink = $milestoneurl ? "$milestoneurl" + my $milestonelink = $milestoneurl ? '' . + html_quote($milestoneurl) . '' : "missing"; $pdesc ||= "missing"; $disallownew = $disallownew ? 'closed' : 'open'; @@ -545,20 +550,20 @@ if ($action eq 'del') { print "\n"; print " Component:\n"; - print " $component"; + print " " . html_quote($component) . ""; print "\n"; print " Component description:\n"; - print " $cdesc"; + print " " . html_light_quote($cdesc) . ""; print "\n"; print " Initial owner:\n"; - print " $initialowner"; + print " " . html_quote($initialowner) . ""; if (Param('useqacontact')) { print "\n"; print " Initial QA contact:\n"; - print " $initialqacontact"; + print " " . html_quote($initialqacontact) . ""; } SendSQL("SELECT count(bug_id) FROM bugs @@ -566,11 +571,11 @@ if ($action eq 'del') { print "\n"; print " Component of product:\n"; - print " $product\n"; + print " " . html_quote($product) . "\n"; print "\n"; print " Description:\n"; - print " $pdesc\n"; + print " " . html_light_quote($pdesc) . "\n"; if (Param('usetargetmilestone')) { print "\n"; @@ -837,7 +842,7 @@ if ($action eq 'update') { exit; } if (TestComponent($product,$component)) { - print "Sorry, component name '$component' is already in use."; + print "Sorry, component name '" . html_quote($component) . "' is already in use."; PutTrailer($localtrailer); exit; } diff --git a/editgroups.cgi b/editgroups.cgi index de7185ae16..2d4911ec74 100755 --- a/editgroups.cgi +++ b/editgroups.cgi @@ -22,7 +22,7 @@ # Joel Peshkin # Jacob Steenhagen # Vlad Dascalu -# Frédéric Buclin +# Frédéric Buclin # Code derived from editowners.cgi and editusers.cgi @@ -222,7 +222,7 @@ unless ($action) { my ($groupid, $name, $desc, $regexp, $isactive, $isbuggroup) = FetchSQLData(); print "\n"; print "" . html_quote($name) . "\n"; - print "" . html_quote($desc) . "\n"; + print "" . html_light_quote($desc) . "\n"; print "" . html_quote($regexp) . " \n"; print ""; print "X" if (($isactive != 0) && ($isbuggroup != 0)); @@ -291,7 +291,7 @@ if ($action eq 'changeform') { } print "Description:"; if ($isbuggroup == 0) { - print html_quote($description); + print html_light_quote($description); } else { print " @@ -356,7 +356,7 @@ if ($action eq 'changeform') { print ""; print ""; print "" . html_quote($grpnam) . ""; - print "" . html_quote($grpdesc) . ""; + print "" . html_light_quote($grpdesc) . ""; print "\n"; } @@ -487,8 +487,8 @@ if ($action eq 'del') { print "\n"; print "\n"; print "$gid\n"; - print "$name\n"; - print "$desc\n"; + print "" . html_quote($name) . "\n"; + print "" . html_light_quote($desc) . "\n"; print "\n"; print "\n"; @@ -529,10 +529,10 @@ group before checking the box.

if (MoreSQLData()) { $cantdelete = 1; print " -This group is tied to the $name product. +This group is tied to the " . html_quote($name) . " product. You cannot delete this group while it is tied to a product.
Delete this group anyway, and make the -$name product publicly visible.
+" . html_quote($name) . " product publicly visible.
"; } @@ -746,14 +746,14 @@ sub confirmRemove { print "return to the Edit Groups page\n"; return; } - print "from group $::FORM{name}.

\n"; + print "from group " . html_quote($::FORM{name}) . ".

\n"; print "Generally, you will only need to do this when upgrading groups "; print "created with Bugzilla versions 2.16 and prior. Use this option "; print "with extreme care and consult the Bugzilla Guide for "; print "further information.

\n"; print "

\n"; - print "\n"; + print "\n"; if ($remove_regexp_only) { print "\n"; diff --git a/editmilestones.cgi b/editmilestones.cgi index dce5e291d3..641e1026d9 100755 --- a/editmilestones.cgi +++ b/editmilestones.cgi @@ -50,7 +50,7 @@ sub CheckProduct ($) } unless (TestProduct $prod) { - print "Sorry, product '$prod' does not exist."; + print "Sorry, product '" . html_quote($prod) . "' does not exist."; PutTrailer(); exit; } @@ -81,7 +81,8 @@ sub CheckMilestone ($$) CheckProduct($prod); unless (TestMilestone $prod,$mile) { - print "Sorry, milestone '$mile' for product '$prod' does not exist."; + print "Sorry, milestone '" . html_quote($mile) . "' for product '" . + html_quote($prod) . "' does not exist."; PutTrailer(); exit; } @@ -209,8 +210,8 @@ unless ($product) { my ($product, $description) = FetchSQLData(); $description ||= "missing"; print "\n"; - print " $product\n"; - print " $description\n"; + print " ", html_quote($product), "\n"; + print " " . html_light_quote($description) . "\n"; } print "\n"; @@ -244,7 +245,7 @@ unless ($action) { my ($milestone,$sortkey,$bugs) = FetchSQLData(); $bugs ||= 'none'; print "\n"; - print " $milestone\n"; + print " ", html_quote($milestone), "\n"; #print " $bugs\n"; print " $sortkey\n"; print " Delete\n"; @@ -314,7 +315,7 @@ if ($action eq 'new') { $sortkey = CheckSortkey($milestone,$sortkey); if (TestMilestone($product,$milestone)) { - print "The milestone '$milestone' already exists. Please press\n"; + print "The milestone '" . html_quote($milestone) . "' already exists. Please press\n"; print "Back and try again.\n"; PutTrailer($localtrailer); exit; @@ -330,7 +331,8 @@ if ($action eq 'new') { unlink "$datadir/versioncache"; print "OK, done.

\n"; - PutTrailer("add another milestone or $localtrailer"); + PutTrailer("add another milestone or $localtrailer"); exit; } @@ -366,10 +368,10 @@ if ($action eq 'del') { print "\n"; print " Product:\n"; - print " $product\n"; + print " " . html_quote($product) . "\n"; print "\n"; print " Milestone:\n"; - print " $milestone\n"; + print " " . html_quote($milestone) . "\n"; print "\n"; print " Bugs:\n"; print " ", $bugs || 'none' , "\n"; @@ -545,7 +547,7 @@ if ($action eq 'update') { exit; } if (TestMilestone($product,$milestone)) { - print "Sorry, milestone '$milestone' is already in use."; + print "Sorry, milestone '" . html_quote($milestone) . "' is already in use."; PutTrailer($localtrailer); exit; } diff --git a/editproducts.cgi b/editproducts.cgi index b3e8fe0417..c9a5f149f3 100755 --- a/editproducts.cgi +++ b/editproducts.cgi @@ -80,7 +80,7 @@ sub CheckProduct ($) } unless (TestProduct $prod) { - print "Sorry, product '$prod' does not exist."; + print "Sorry, product '" . html_quote($prod) . "' does not exist."; PutTrailer(); exit; } @@ -231,8 +231,8 @@ unless ($action) { $disallownew = $disallownew ? 'closed' : 'open'; $bugs ||= 'none'; print "\n"; - print " $product\n"; - print " $description\n"; + print " ", html_quote($product), "\n"; + print " " . html_light_quote($description) . "\n"; print " $disallownew\n"; print " $votesperuser\n"; print " $maxvotesperbug\n"; @@ -308,7 +308,7 @@ if ($action eq 'new') { exit; } if (TestProduct($product)) { - print "The product '$product' already exists. Please press\n"; + print "The product '" . html_quote($product) . "' already exists. Please press\n"; print "Back and try again.\n"; PutTrailer($localtrailer); exit; @@ -317,7 +317,7 @@ if ($action eq 'new') { my $version = trim($::FORM{version} || ''); if ($version eq '') { - print "You must enter a version for product '$product'. Please press\n"; + print "You must enter a version for product '" . html_quote($product) . "'. Please press\n"; print "Back and try again.\n"; PutTrailer($localtrailer); exit; @@ -460,7 +460,8 @@ if ($action eq 'del') { FROM products WHERE name=" . SqlQuote($product)); my ($product_id, $description, $milestoneurl, $disallownew) = FetchSQLData(); - my $milestonelink = $milestoneurl ? "$milestoneurl" + my $milestonelink = $milestoneurl ? '' . + html_quote($milestoneurl) . '' : "missing"; $description ||= "description missing"; $disallownew = $disallownew ? 'closed' : 'open'; @@ -472,11 +473,11 @@ if ($action eq 'del') { print "\n"; print " Product:\n"; - print " $product\n"; + print " " . html_quote($product) . "\n"; print "\n"; print " Description:\n"; - print " $description\n"; + print " " . html_light_quote($description) . "\n"; if (Param('usetargetmilestone')) { print "\n"; @@ -500,8 +501,8 @@ if ($action eq 'del') { while ( MoreSQLData() ) { my ($component, $description) = FetchSQLData(); $description ||= "description missing"; - print "$component:"; - print "$description\n"; + print "" . html_quote($component) . ":"; + print "" . html_light_quote($description) . "\n"; } print "\n"; } else { @@ -520,7 +521,7 @@ if ($action eq 'del') { while ( MoreSQLData() ) { my ($version) = FetchSQLData(); print "
" if $br; - print $version; + print html_quote($version); $br = 1; } } else { @@ -543,7 +544,7 @@ if ($action eq 'del') { while ( MoreSQLData() ) { my ($milestone) = FetchSQLData(); print "
" if $br; - print $milestone; + print html_quote($milestone); $br = 1; } } else { @@ -672,7 +673,7 @@ if ($action eq 'delete') { SendSQL("DELETE FROM products WHERE id=$product_id"); - print "Product '$product' deleted.
\n"; + print "Product '" . html_quote($product) . "' deleted.
\n"; unlink "$datadir/versioncache"; PutTrailer($localtrailer); @@ -718,8 +719,8 @@ if ($action eq 'edit') { while ( MoreSQLData() ) { my ($component, $description) = FetchSQLData(); $description ||= "description missing"; - print "$component:"; - print "$description\n"; + print "" . html_quote($component) . ":"; + print "" . html_light_quote($description) . "\n"; } print "\n"; } else { @@ -739,7 +740,7 @@ if ($action eq 'edit') { while ( MoreSQLData() ) { my ($version) = FetchSQLData(); print "
" if $br; - print $version; + print html_quote($version); $br = 1; } } else { @@ -762,7 +763,7 @@ if ($action eq 'edit') { while ( MoreSQLData() ) { my ($milestone) = FetchSQLData(); print "
" if $br; - print $milestone; + print html_quote($milestone); $br = 1; } } else { @@ -1168,7 +1169,7 @@ if ($action eq 'update') { "WHERE value = " . SqlQuote($defaultmilestone) . " AND product_id = $product_id"); if (!FetchOneColumn()) { - print "Sorry, the milestone $defaultmilestone must be defined first."; + print "Sorry, the milestone " . html_quote($defaultmilestone) . " must be defined first."; PutTrailer($localtrailer); exit; } @@ -1188,7 +1189,7 @@ if ($action eq 'update') { exit; } if (TestProduct($product)) { - print "Sorry, product name '$product' is already in use."; + print "Sorry, product name '" . html_quote($product) . "' is already in use."; PutTrailer($localtrailer); exit; } @@ -1217,7 +1218,8 @@ if ($action eq 'update') { my ($who, $id) = (@$ref); RemoveVotes($id, $who, "The rules for voting on this product has changed;\nyou had too many votes for a single bug."); my $name = DBID_to_name($who); - print qq{
Removed votes for bug $id from $name\n}; + print "
Removed votes for bug $id from " . + html_quote($name) . "\n"; } } @@ -1249,7 +1251,8 @@ if ($action eq 'update') { RemoveVotes($id, $who, "The rules for voting on this product has changed; you had too many\ntotal votes, so all votes have been removed."); my $name = DBID_to_name($who); - print qq{
Removed votes for bug $id from $name\n}; + print "
Removed votes for bug $id from " . + html_quote($name) . "\n"; } } } @@ -1346,10 +1349,10 @@ if ($action eq 'editgroupcontrols') { print "\n"; my ($groupid, $groupname, $entry, $membercontrol, $othercontrol, $canedit) = FetchSQLData(); - print "\n"; + print "\n"; print " \n"; - print " $groupname\n"; + print " " . html_quote($groupname) . "\n"; print " \n"; $entry |= 0; print " $groupname\n"; + print "\n"; } print "

\n"; print "\n"; print "\n"; print "\n"; - print "\n"; + print "\n"; print "

\n"; print "

note: Any group controls Set to NA/NA with no other checkboxes "; print "will automatically be removed from the panel the next time "; diff --git a/editusers.cgi b/editusers.cgi index c7701656bd..2ec9e87e8a 100755 --- a/editusers.cgi +++ b/editusers.cgi @@ -76,7 +76,7 @@ sub CheckUser ($) } unless (TestUser $user) { - print "Sorry, user '$user' does not exist."; + print "Sorry, user '" . html_quote($user) . "' does not exist."; PutTrailer(); exit; } @@ -87,6 +87,7 @@ sub CheckUser ($) sub EmitElement ($$) { my ($name, $value) = (@_); + $name = html_quote($name); $value = value_quote($value); if ($editall) { print qq{\n}; @@ -193,7 +194,7 @@ sub EmitFormElements ($$$$) print ']' if ($isderived); print '*' if ($isregexp); print ""; - print ucfirst($name) . ": $description\n"; + print html_quote(ucfirst($name)) . ": " . html_light_quote($description) . "\n"; } } print "\n"; @@ -466,7 +467,7 @@ if ($action eq 'new') { exit; } if (!ValidateNewUser($user)) { - print "The user '$user' does already exist. Please press\n"; + print "The user '" . html_quote($user) . "' does already exist. Please press\n"; print "Back and try again.\n"; PutTrailer($localtrailer); exit; @@ -490,7 +491,7 @@ if ($action eq 'new') { SendSQL("SELECT last_insert_id()"); my ($newuserid) = FetchSQLData(); - print "To change ${user}'s permissions, go back and " . + print "To change " . html_quote($user) . "'s permissions, go back and " . "edit this user."; print "

\n"; @@ -535,7 +536,7 @@ if ($action eq 'del') { print "\n"; print " Login name:\n"; - print " $user\n"; + print " " . html_quote($user) . "\n"; print "\n"; print " Real name:\n"; @@ -554,7 +555,7 @@ if ($action eq 'del') { while ( MoreSQLData() ) { my ($name) = FetchSQLData(); print "
\n" if $found; - print ucfirst $name; + print html_quote(ucfirst($name)); $found = 1; } print "none" unless $found; @@ -580,7 +581,7 @@ if ($action eq 'del') { my ($product, $component) = FetchSQLData(); print "$product: $component"; + "&action=edit\">" . html_quote($product) . ": " . html_quote($component) . ""; $found = 1; $nodelete = 'initial bug owner'; } @@ -605,7 +606,7 @@ if ($action eq 'del') { my ($product, $component) = FetchSQLData(); print "$product: $component"; + "&action=edit\">" . html_quote($product) . ": " . html_quote($component) . ""; $found = 1; $nodelete = 'initial QA contact'; } @@ -615,7 +616,7 @@ if ($action eq 'del') { if ($nodelete) { - print "

You can't delete this user because '$user' is an $nodelete ", + print "

You can't delete this user because '" . html_quote($user) . "' is an $nodelete ", "for at least one product."; PutTrailer($localtrailer); exit; @@ -628,7 +629,7 @@ if ($action eq 'del') { print "

\n"; print "\n"; print "\n"; - print "\n"; + print "\n"; print "
"; PutTrailer($localtrailer); @@ -700,8 +701,8 @@ if ($action eq 'edit') { EmitFormElements($thisuserid, $user, $realname, $disabledtext); print "\n"; - print "\n"; - print "\n"; + print "\n"; + print "\n"; print "\n"; print "\n"; @@ -720,7 +721,7 @@ if ($action eq 'edit') { print "
\n"; print "\n"; print "\n"; - print "\n"; + print "\n"; print "
"; } @@ -777,10 +778,10 @@ if ($action eq 'update') { SendSQL("INSERT INTO user_group_map (user_id, group_id, isbless, grant_type) VALUES ($thisuserid, $groupid, 0," . GRANT_DIRECT . ")"); - print "Added user to group $name
\n"; + print "Added user to group " . html_quote($name) . "
\n"; push(@grpadd, $name); } else { - print "Dropped user from group $name
\n"; + print "Dropped user from group " . html_quote($name) . "
\n"; push(@grpdel, $name); } PopGlobalSQLState(); @@ -797,9 +798,9 @@ if ($action eq 'update') { SendSQL("INSERT INTO user_group_map (user_id, group_id, isbless, grant_type) VALUES ($thisuserid, $groupid, 1," . GRANT_DIRECT . ")"); - print "Granted user permission to bless group $name
\n"; + print "Granted user permission to bless group " . html_quote($name) . "
\n"; } else { - print "Revoked user's permission to bless group $name
\n"; + print "Revoked user's permission to bless group " . html_quote($name) . "
\n"; } PopGlobalSQLState(); @@ -859,7 +860,7 @@ if ($action eq 'update') { exit; } if (TestUser($user)) { - print "Sorry, user name '$user' is already in use."; + print "Sorry, user name '" . html_quote($user) . "' is already in use."; $userold = url_quote($userold); $localtrailer =~ s/XXX/$userold/; push @localtrailers, $localtrailer; diff --git a/editversions.cgi b/editversions.cgi index c426891edf..b74338c896 100755 --- a/editversions.cgi +++ b/editversions.cgi @@ -63,7 +63,7 @@ sub CheckProduct ($) } unless (TestProduct $prod) { - print "Sorry, product '$prod' does not exist."; + print "Sorry, product '" . html_quote($prod) . "' does not exist."; PutTrailer(); exit; } @@ -94,7 +94,8 @@ sub CheckVersion ($$) CheckProduct($prod); unless (TestVersion $prod,$ver) { - print "Sorry, version '$ver' for product '$prod' does not exist."; + print "Sorry, version '" . html_quote($ver) . "' for product '" . + html_quote($prod) . "' does not exist."; PutTrailer(); exit; } @@ -206,8 +207,8 @@ unless ($product) { $description ||= "missing"; $bugs ||= "none"; print "\n"; - print " $product\n"; - print " $description\n"; + print " ", html_quote($product), "\n"; + print " " . html_light_quote($description) . "\n"; print " $bugs\n"; #print " Edit\n"; } @@ -239,7 +240,7 @@ unless ($action) { while ( MoreSQLData() ) { my $version = FetchOneColumn(); print "\n"; - print " $version\n"; + print " " . html_quote($version) . "\n"; #print " $bugs\n"; print " Delete\n"; print ""; @@ -305,7 +306,7 @@ if ($action eq 'new') { exit; } if (TestVersion($product,$version)) { - print "The version '$version' already exists. Please press\n"; + print "The version '" . html_quote($version) . "' already exists. Please press\n"; print "Back and try again.\n"; PutTrailer($localtrailer); exit; @@ -321,7 +322,8 @@ if ($action eq 'new') { unlink "$datadir/versioncache"; print "OK, done.

\n"; - PutTrailer("add another version or $localtrailer"); + PutTrailer("add another version or $localtrailer"); exit; } @@ -352,10 +354,10 @@ if ($action eq 'del') { print "\n"; print " Product:\n"; - print " $product\n"; + print " " . html_quote($product) . "\n"; print "\n"; print " Version:\n"; - print " $version\n"; + print " " . html_quote($version) . "\n"; print "\n"; print " Bugs:\n"; print " ", $bugs || 'none' , "\n"; @@ -512,7 +514,7 @@ if ($action eq 'update') { exit; } if (TestVersion($product,$version)) { - print "Sorry, version '$version' is already in use."; + print "Sorry, version '" . html_quote($version) . "' is already in use."; PutTrailer($localtrailer); exit; } diff --git a/globals.pl b/globals.pl index e01ef23fb2..d403a03f8f 100644 --- a/globals.pl +++ b/globals.pl @@ -915,7 +915,8 @@ sub quoteUrls { my $tmp; # non-mailto protocols - my $protocol_re = qr/(afs|cid|ftp|gopher|http|https|irc|mid|news|nntp|prospero|telnet|view-source|wais)/i; + my $safe_protocols = join('|', SAFE_PROTOCOLS); + my $protocol_re = qr/($safe_protocols)/i; $text =~ s~\b(${protocol_re}: # The protocol: [^\s<>\"]+ # Any non-whitespace diff --git a/t/008filter.t b/t/008filter.t index 02456e73a5..71006c85ce 100644 --- a/t/008filter.t +++ b/t/008filter.t @@ -219,7 +219,7 @@ sub directive_ok { # Note: If a single directive prints two things, and only one is # filtered, we may not catch that case. return 1 if $directive =~ /FILTER\ (html|csv|js|url_quote|css_class_quote| - ics|quoteUrls|time|uri|xml|lower| + ics|quoteUrls|time|uri|xml|lower|html_light| obsolete|inactive|closed|unitconvert| none)\b/x; diff --git a/template/en/default/account/prefs/permissions.html.tmpl b/template/en/default/account/prefs/permissions.html.tmpl index 46d4559ec8..77cc70999f 100644 --- a/template/en/default/account/prefs/permissions.html.tmpl +++ b/template/en/default/account/prefs/permissions.html.tmpl @@ -42,8 +42,8 @@ [% FOREACH bit_description = has_bits %] - - + + [% END %]
[% bit_description.name %][% bit_description.desc %][% bit_description.name FILTER html %][% bit_description.desc FILTER html_light %]
@@ -63,8 +63,8 @@ [% FOREACH bit_description = set_bits %] - - + + [% END %]
[% bit_description.name %][% bit_description.desc %][% bit_description.name FILTER html %][% bit_description.desc FILTER html_light %]
diff --git a/template/en/default/admin/keywords/list.html.tmpl b/template/en/default/admin/keywords/list.html.tmpl index fbfc921429..8df11262fe 100755 --- a/template/en/default/admin/keywords/list.html.tmpl +++ b/template/en/default/admin/keywords/list.html.tmpl @@ -18,12 +18,13 @@ # # Contributor(s): Terry Weissman # Vlad Dascalu - # Jouni Heikniemi #%] [%# INTERFACE: + # max_table_size: number. Determines the maximum number of + # rows in each keywords table. # keywords: array with keyword objects having the properties: - # - id: number. The ID of the keyword. + # - keyword_id: number. The ID of the keyword. # - name: string. The name of the keyword. # - description: string. The description of the keyword. # - bug_count: number. The number of bugs with the keyword. @@ -35,35 +36,62 @@ title = "Select keyword" %] -[% columns = [ - { - name => "name" - heading => "Edit keyword..." - contentlink => "editkeywords.cgi?action=edit&id=%id%" - }, - { - name => "description" - heading => "Description" - }, - { - name => "bug_count" - heading => "Bugs" - align => "right" - }, - { - heading => "Action" - content => "Delete" - contentlink => "editkeywords.cgi?action=delete&id=%id%" - } - ] -%] +[% max_table_size = 50 %] -[% PROCESS admin/table.html.tmpl - columns = columns - data = keywords - footer = footer_row -%] +[% BLOCK table_header %] + + + + + + + +[% END %] + +[% BLOCK table_footer %] +
Edit keyword ...Description[% terms.Bugs %]Action
+[% END %] + +[% PROCESS table_header %] + +[% FOREACH keyword = keywords %] + [% IF !loop.first() && loop.count() % max_table_size == 1 %] + [% PROCESS table_header %] + [% END %] + + + + [% keyword.name FILTER html %] + + + [% IF keyword.description %] + [% keyword.description FILTER html_light %] + [% ELSE %] + missing + [% END %] + + + [% IF keyword.bug_count %] + [% keyword.bug_count %] + [% ELSE %] + none + [% END %] + + + Delete + + + + [% IF !loop.last() && loop.count() % max_table_size == 0 %] + [% PROCESS table_footer %] + [% END %] +[% END %] + + + Add a new keyword + Add + -

Add a new keyword

+[% PROCESS table_footer %] [% PROCESS global/footer.html.tmpl %] diff --git a/template/en/default/bug/create/create.html.tmpl b/template/en/default/bug/create/create.html.tmpl index 6745cd6f19..49162f3f8f 100644 --- a/template/en/default/bug/create/create.html.tmpl +++ b/template/en/default/bug/create/create.html.tmpl @@ -306,7 +306,7 @@ function set_assign_to() { -
+
[% END %]
[% END %] diff --git a/template/en/default/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl index fda1826964..d67afc8c8c 100644 --- a/template/en/default/bug/edit.html.tmpl +++ b/template/en/default/bug/edit.html.tmpl @@ -487,7 +487,7 @@ - [% group.description %] + [% group.description FILTER html_light %]
[% END %] [% END %] diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl index 20c5a78236..08f885175b 100644 --- a/template/en/default/filterexceptions.pl +++ b/template/en/default/filterexceptions.pl @@ -91,7 +91,6 @@ 'reports/components.html.tmpl' => [ 'numcols', - 'comp.description', ], 'reports/duplicates-table.html.tmpl' => [ @@ -115,7 +114,6 @@ ], 'reports/keywords.html.tmpl' => [ - 'keyword.description', 'keyword.bugcount', ], @@ -196,16 +194,10 @@ 'list/edit-multiple.html.tmpl' => [ 'group.id', - 'group.description', - 'group.description FILTER inactive', 'knum', 'menuname', ], -'list/list-simple.html.tmpl' => [ - 'title', -], - 'list/list.html.tmpl' => [ 'buglist', ], @@ -241,7 +233,6 @@ 'global/choose-product.html.tmpl' => [ 'target', - 'proddesc.$p', ], # You are not permitted to add any values here. Everything in this file should @@ -328,7 +319,6 @@ 'bug.bug_id', 'bug.votes', 'group.bit', - 'group.description', 'dep.title', 'dep.fieldname', 'accesskey', @@ -404,7 +394,6 @@ 'bug/create/create.html.tmpl' => [ 'g.bit', - 'g.description', 'sel.name', 'sel.description', ], @@ -536,11 +525,6 @@ 'reason.description', ], -'account/prefs/permissions.html.tmpl' => [ - 'bit_description.name', - 'bit_description.desc', -], - 'account/prefs/prefs.html.tmpl' => [ 'tab.name', 'tab.description', diff --git a/template/en/default/global/choose-product.html.tmpl b/template/en/default/global/choose-product.html.tmpl index 138c354ae7..ad9867919b 100644 --- a/template/en/default/global/choose-product.html.tmpl +++ b/template/en/default/global/choose-product.html.tmpl @@ -48,7 +48,7 @@ [% IF proddesc.$p %] -  [% proddesc.$p %] +  [% proddesc.$p FILTER html_light %] [% END %] [% END %] diff --git a/template/en/default/list/edit-multiple.html.tmpl b/template/en/default/list/edit-multiple.html.tmpl index eda6d2fa18..0b1f75e141 100644 --- a/template/en/default/list/edit-multiple.html.tmpl +++ b/template/en/default/list/edit-multiple.html.tmpl @@ -217,11 +217,8 @@ [% END %] - [% IF group.isactive %] - [% group.description %] - [% ELSE %] - [% group.description FILTER inactive %] - [% END %] + [% SET inactive = !group.isactive %] + [% group.description FILTER html_light FILTER inactive(inactive) %] diff --git a/template/en/default/list/list-simple.html.tmpl b/template/en/default/list/list-simple.html.tmpl index 9c37e77f95..a25f42c175 100644 --- a/template/en/default/list/list-simple.html.tmpl +++ b/template/en/default/list/list-simple.html.tmpl @@ -30,8 +30,6 @@ [%############################################################################%] [% DEFAULT title = "$terms.Bug List" %] -[% title = title FILTER html %] - [%############################################################################%] [%# Bug Table #%] @@ -40,7 +38,7 @@ - [% title %] + [% title FILTER html %] diff --git a/template/en/default/reports/components.html.tmpl b/template/en/default/reports/components.html.tmpl index ae969171bf..c3a3ae5162 100644 --- a/template/en/default/reports/components.html.tmpl +++ b/template/en/default/reports/components.html.tmpl @@ -23,7 +23,7 @@ # product: string. The product this is the components list for. # components: List of hashes. May be empty. Each hash has four members: # name: string. Name of the component. - # description: string. Description of the component. May contain HTML. + # description: string. Description of the component. Can contain some limited HTML code. # initialowner: string. Component's initial owner. # initialqacontact: string. Component's initial QA contact. #%] @@ -93,7 +93,7 @@ - [% comp.description %] + [% comp.description FILTER html_light %] [% END %] diff --git a/template/en/default/reports/keywords.html.tmpl b/template/en/default/reports/keywords.html.tmpl index bd52cf6eca..10bfe1e873 100644 --- a/template/en/default/reports/keywords.html.tmpl +++ b/template/en/default/reports/keywords.html.tmpl @@ -22,7 +22,7 @@ [%# INTERFACE: # keywords: array of hashes. May be empty. Each has has three members: # name: the name of the keyword - # description: keyword description. May be HTML. + # description: keyword description. Can contain some limited HTML code. # bugcount: number of bugs with that keyword # caneditkeywords: boolean. True if this user can edit keywords %] @@ -52,7 +52,7 @@ [% keyword.name FILTER html %] - [% keyword.description %] + [% keyword.description FILTER html_light %] [% IF keyword.bugcount > 0 %]