297845 - Several Template Toolkit filters are being used bug not defined

Status: RESOLVED INVALID

(Bugzilla :: User Interface, defect, P1)

Description

[A. Karl Kornel]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US) AppleWebKit/125.4 (KHTML, like Gecko, Safari) OmniWeb/v563.34
Build Identifier: 

There are a number of Template Toolkit filters that match the following criteria:

* They are not defined (at least, not in Bugzilla/Template.pm)
* They are being used in at least one template
* They are listed in t/008filters.t, which is why the automatic tests aren't picking it up.

Reproducible: Always

Steps to Reproduce:




Thus far 'uri' and 'lower' are known to be undefined, but used.  Any others?

Once the bug is confirmed, I can take it and search through all the templates for occurrences of the 
undefined filters.  I'll also update the test file.

Comment 1

[A. Karl Kornel]

'uri' and 'lower' are the only filters in t/008filters.t that are not defined in Bugzilla/Template.pm.

Comment 2

[Frédéric Buclin]

confirmed! I reached to run arbitrary code in my browser.

Comment 3

[A. Karl Kornel]

Attachment: Patch for test suite

Modifies the standard test suite to no longer allow 'uri' or 'lower' filters.

Comment 4

[Frédéric Buclin]

Here is a simple testcase:

1) Use target milestones (editparams.cgi)
2) Edit any product
3) Enter the following line in the "URL describing milestones for this product"
(you have to add the double quotes as noted here):
"<script type='text/javascript'>alert('hello');</script>"
4) Save this change
5) Try to delete this product

Result: "hello" is displayed. I could run any arbitrary code, e.g. reading cookies.

Comment 5

[Frédéric Buclin]

Comment on attachment 186403 [details] [diff] [review]
Patch for test suite

This part is correct, but you have to fixed the 5 affected templates too.

Comment 6

[Frédéric Buclin]

Moreover, we should have 'html => sub { return $_ },' in 004template.t IMO. Any
reason not to have it there? (I know it's not related to this bug, but while we
are fixing filters...)

Comment 7

[A. Karl Kornel]

Attachment: Patches 5 template files + one test script, patch on top of tip.

There's at least one additional file in 2.18.1 that needs to be patched but
does not have the problem in the tip copy, so this patch probably won't work on
2.18.1 directly.  Once this is approved I'll start on patching 2.18.1.

Comment 8

[Frédéric Buclin]

2.16.10 seems to be affected too.

Comment 9

[Frédéric Buclin]

Comment on attachment 186410 [details] [diff] [review]
Patches 5 template files + one test script, patch on top of tip.

>Index: template/en/default/list/quips.html.tmpl

>-              <input type="checkbox" name="quipid_[% quipid FILTER uri%]"
>-                     id="quipid_[% quipid FILTER uri%]"
>+              <input type="checkbox" name="quipid_[% quipid FILTER url_quote %]"
>+                     id="quipid_[% quipid FILTER url_quote %]"
>                      [%- ' checked="checked"' IF quips.$quipid.approved %]>

Nit: we could add quipid to filterexceptions.pl as it comes directly from the
DB, or even mark it as FILTER none or FILTER html. Looking at other templates,
I found all these different combinations, so this one is certainly not the
worst one. ;)


>Index: template/en/default/account/prefs/prefs.html.tmpl

>-      [% current_tab.description FILTER lower %] have been saved.
>+      [% current_tab.description FILTER html %] have been saved.

Nit: Looking at it closer, FILTER none would work too. But this solution is
safe.

r=LpSolit

Comment 10

[A. Karl Kornel]

Attachment: Patches 4+1 template files + one test script, on top of 2.18.1

Patches 5 template files + one test script, patch on top of 2.18.1.  It should
be noted that there's one file in this patch that did not need patching in tip,
and there's one file in tip that needed patching, but wasn't present in 2.18.1.


Once this is approved I'll start on patching 2.16.whatever.

Comment 11

[Frédéric Buclin]

bah... we just found that 'uri' and 'lower' were built-in filters, see
http://template-toolkit.org/docs/default/Modules/Template/Filters.html#uri

shame on me! :-[

Can someone confirm this and mark the bug as invalid?

/me is really tired...

Comment 12

[Frédéric Buclin]

Comment on attachment 186410 [details] [diff] [review]
Patches 5 template files + one test script, patch on top of tip.

removing my review, the bug is invalid

Comment 13

[Frédéric Buclin]

ok, additional tests show that this bug is invalid. My testcase is due to the
milestone URL not being filtered in the non-templatized editproducts.cgi file.

Sorry for the bugspam. :-[