[% IF !h1.defined %][% h1 = title %][% END %] <title>[% title FILTER html %]</title> [% IF h1 %] <h1>[% h1 %]</h1> [% END %] If h1 is not defined, which is the case on most pages, h1 has the same content as title, but only title is filtered, h1 is not (see above). I was reviewing bug 330290 and I wondered why only part of the user identity was displayed in h1 while it was correctly displayed in the title. The reason was because we had: <title>Edit user Guest 4 &lt;guest4&#64;bugzilla.org&gt;</title> <div id="header"> <h1>Edit user Guest 4 <guest4@bugzilla.org></h1> </div> "<" and ">" have not been filtered in h1 and Firefox ignored this "tag". If I wrote "script ..." instead of guest4, the JS code would have been executed.

Also discussed in bug#317126 Need to firm up the interface to headers.html.tmpl. Either callers filter all params, or headers template filters all, or we don't default <h1>, and require it from callers Changes will be required in a few callers.

That makes this a sec bug.

For security reasons, it's safer if global/headers.html.tmpl filters titles and H1-H3 itself.

Hmm, can title tag contain HTML formatting? I thought it was pure text.

(In reply to comment #4) > Hmm, can title tag contain HTML formatting? I thought it was pure text. > It is, that's why <title> must be filtered in all cases. Consequently, we have no other choices than filtering H1-H3 too. And to be sure this is done, we should do it in a centralized way.

(In reply to comment #5) > (In reply to comment #4) > > Hmm, can title tag contain HTML formatting? I thought it was pure text. > It is, that's why <title> must be filtered in all cases. Consequently, we have Then why some templates are trying to output HTML to title anyway? Like your patch in attachment 216459 [details] [diff] [review] on bug 44595. Hmm, maybe they are trying to provide value for h1 (that I think can have some markup, but I'm not sure). That's just stupid. They should use h1 specifically and provide two different values (one for title and one for h1). This begins to sound like I want title and h1 decoupled on the header template. :)

I just did a quick audit of some of our templates on tip, and I realized that most titles and H1-H3 strings were filtered in templates themselves, before being passed to header.html.tmpl. In comment 3 and even today on IRC, I suggested to change this behavior and do the filtering in header.html.tmpl. But now I change my mind for the following reasons: 1. Looks like > 95% of all templates pass filtered strings. So fixing the remaining 5% is much less invasive and prone to regressions. 2. If we decided to pass unfiltered strings to header.html.tmpl, filterexceptions.pl (and 008filter.t) would complain that these fields are... unfiltered, forcing us to add boring "FILTER none" almost everywhere, or even worse: forcing us to add these fields in filterexceptions.pl, which I consider as a potentially security problem, as 008filter.t wouldn't complain anymore if we forget to filter a directive elsewhere in the template. So this last suggestion is not acceptable. And we can as well write "FILTER html" instead of "FILTER none" directly in templates. The number of characters is exactly the same! ;) I want this bug fixed asap, in addition to bug 206037. This problem is even more visible on 2.18 and 2.20, where title filtering is almost inexistant.

The filter test wouldn't complain that a variable is unfiltered inside of a PROCESS statement, or inside of a SET statement, right? I think the only reason to require callers to filter h1/title/h2 is if we *want* to have HTML code in them. Otherwise, if they should never have HTML, seems like they should be filtered inside header.html.tmpl. I see at least in our 2.20 example that this is exploitable on an admin page, so that makes this a valid sec. blocker.

For 2.18, I forced PutHeader() in CGI.pl to filter title, h1 and h2 itself for not yet templatized pages. This is way much less invasive than fixing all these edit*.cgi scripts. And in opposition to 2.20 and newer, URLs are never passed to headers so it's safe to do it here and doesn't break current behavior.

This bug is scheduled for the same release as bug 321556, but they depend on each other. As this is a sec bug, and as such needs to go in more or less at time of release, leaving no time for bug 321556, it'd make sense to reverse the dependency--but this would hit this bug's tip patch with bad rottage. Shall we reverse the dependency anyway?

Marc, please no. We would like to start QA tests next week, assuming all security patches will be reviewed meanwhile. Anyway, I won't prevent you from working on your skin patch. If yours is checked in before this one is reviewed, I will unbitrot it (I have no other choices anyway). So let's keep the dependencies unchanged, but feel free to work on your patch.

Comment on attachment 231308 [details] [diff] [review] patch for tip, v1 This looks good as is. Although there isn't a dependency set, I seem to recall someone saying this depended on the patch on bug 206037, and I had both that patch and this one applied in tandem for review purposes.

As nearly as I can tell from looking at bonsai, every version of Bugzilla that's ever had templates was affected by this bug, meaning it goes back to about 2.15. So I'm setting the version field appropriately. (This stuff is needed for the security advisory.)

tip: Checking in template/en/default/filterexceptions.pl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v <-- filterexceptions.pl new revision: 1.80; previous revision: 1.79 done Checking in template/en/default/admin/flag-type/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/flag-type/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.21; previous revision: 1.20 done Checking in template/en/default/admin/groups/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.8; previous revision: 1.7 done Checking in template/en/default/admin/groups/remove.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/remove.html.tmpl,v <-- remove.html.tmpl new revision: 1.2; previous revision: 1.1 done Checking in template/en/default/admin/users/confirm-delete.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/users/confirm-delete.html.tmpl,v <-- confirm-delete.html.tmpl new revision: 1.13; previous revision: 1.12 done Checking in template/en/default/admin/users/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/users/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.6; previous revision: 1.5 done Checking in template/en/default/bug/show.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show.html.tmpl,v <-- show.html.tmpl new revision: 1.15; previous revision: 1.14 done Checking in template/en/default/bug/create/create.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v <-- create.html.tmpl new revision: 1.68; previous revision: 1.67 done Checking in template/en/default/global/header.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/header.html.tmpl,v <-- header.html.tmpl new revision: 1.47; previous revision: 1.46 done Checking in template/en/default/reports/duplicates.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/duplicates.html.tmpl,v <-- duplicates.html.tmpl new revision: 1.17; previous revision: 1.16 done 2.22: Checking in template/en/default/filterexceptions.pl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v <-- filterexceptions.pl new revision: 1.61.2.2; previous revision: 1.61.2.1 done Checking in template/en/default/admin/flag-type/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/flag-type/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.14.2.2; previous revision: 1.14.2.1 done Checking in template/en/default/admin/groups/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.5.6.2; previous revision: 1.5.6.1 done Checking in template/en/default/admin/groups/remove.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/remove.html.tmpl,v <-- remove.html.tmpl new revision: 1.1.10.1; previous revision: 1.1 done Checking in template/en/default/admin/users/confirm-delete.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/users/confirm-delete.html.tmpl,v <-- confirm-delete.html.tmpl new revision: 1.7.2.3; previous revision: 1.7.2.2 done Checking in template/en/default/admin/users/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/users/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.2.2.2; previous revision: 1.2.2.1 done Checking in template/en/default/bug/show.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show.html.tmpl,v <-- show.html.tmpl new revision: 1.11.2.1; previous revision: 1.11 done Checking in template/en/default/global/header.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/header.html.tmpl,v <-- header.html.tmpl new revision: 1.40.2.1; previous revision: 1.40 done Checking in template/en/default/reports/components.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/components.html.tmpl,v <-- components.html.tmpl new revision: 1.9.4.2; previous revision: 1.9.4.1 done Checking in template/en/default/reports/duplicates.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/duplicates.html.tmpl,v <-- duplicates.html.tmpl new revision: 1.15.2.1; previous revision: 1.15 done 2.20.2: Checking in editproducts.cgi; /cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v <-- editproducts.cgi new revision: 1.85.2.10; previous revision: 1.85.2.9 done Checking in template/en/default/filterexceptions.pl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v <-- filterexceptions.pl new revision: 1.43.2.6; previous revision: 1.43.2.5 done Checking in template/en/default/admin/flag-type/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/flag-type/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.11.4.2; previous revision: 1.11.4.1 done Checking in template/en/default/admin/groups/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.5.4.2; previous revision: 1.5.4.1 done Checking in template/en/default/admin/groups/remove.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/remove.html.tmpl,v <-- remove.html.tmpl new revision: 1.1.8.1; previous revision: 1.1 done Checking in template/en/default/admin/products/groupcontrol/confirm-edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/groupcontrol/confirm-edit.html.tmpl,v <-- confirm-edit.html.tmpl new revision: 1.5.10.2; previous revision: 1.5.10.1 done Checking in template/en/default/admin/users/confirm-delete.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/users/confirm-delete.html.tmpl,v <-- confirm-delete.html.tmpl new revision: 1.4.2.6; previous revision: 1.4.2.5 done Checking in template/en/default/admin/users/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/users/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.1.4.2; previous revision: 1.1.4.1 done Checking in template/en/default/bug/show.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show.html.tmpl,v <-- show.html.tmpl new revision: 1.10.6.1; previous revision: 1.10 done Checking in template/en/default/global/header.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/header.html.tmpl,v <-- header.html.tmpl new revision: 1.39.4.2; previous revision: 1.39.4.1 done Checking in template/en/default/list/list.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list.html.tmpl,v <-- list.html.tmpl new revision: 1.37.2.3; previous revision: 1.37.2.2 done Checking in template/en/default/reports/components.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/components.html.tmpl,v <-- components.html.tmpl new revision: 1.9.2.2; previous revision: 1.9.2.1 done Checking in template/en/default/reports/duplicates.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/duplicates.html.tmpl,v <-- duplicates.html.tmpl new revision: 1.14.10.1; previous revision: 1.14 done 2.18.5: Checking in CGI.pl; /cvsroot/mozilla/webtools/bugzilla/Attic/CGI.pl,v <-- CGI.pl new revision: 1.211.2.11; previous revision: 1.211.2.10 done Checking in editgroups.cgi; /cvsroot/mozilla/webtools/bugzilla/editgroups.cgi,v <-- editgroups.cgi new revision: 1.38.2.4; previous revision: 1.38.2.3 done Checking in template/en/default/filterexceptions.pl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v <-- filterexceptions.pl new revision: 1.16.2.4; previous revision: 1.16.2.3 done Checking in template/en/default/admin/flag-type/edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/flag-type/edit.html.tmpl,v <-- edit.html.tmpl new revision: 1.6.2.5; previous revision: 1.6.2.4 done Checking in template/en/default/admin/products/groupcontrol/confirm-edit.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/groupcontrol/confirm-edit.html.tmpl,v <-- confirm-edit.html.tmpl new revision: 1.5.2.1; previous revision: 1.5 done Checking in template/en/default/bug/show.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show.html.tmpl,v <-- show.html.tmpl new revision: 1.6.2.5; previous revision: 1.6.2.4 done Checking in template/en/default/global/header.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/header.html.tmpl,v <-- header.html.tmpl new revision: 1.24.2.6; previous revision: 1.24.2.5 done Checking in template/en/default/list/list.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list.html.tmpl,v <-- list.html.tmpl new revision: 1.25.2.3; previous revision: 1.25.2.2 done Checking in template/en/default/reports/components.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/components.html.tmpl,v <-- components.html.tmpl new revision: 1.8.2.2; previous revision: 1.8.2.1 done Checking in template/en/default/reports/duplicates.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/duplicates.html.tmpl,v <-- duplicates.html.tmpl new revision: 1.14.2.1; previous revision: 1.14 done

Security Advisory has been sent, so this bug is no longer private.