On 2023-09-29 19:55, bawolff wrote:
> This is clearly yielding some interesting results.
>
> One of the patterns i've noticed is that several of the examples seem to
> involve mustache templates. I think there are two reasons for this:
>
> * mustache templates cannot currently be checked by phan-taint-check
> * Because they are a separate file, the escaping is now fairly far away
> from the context where the variable is used. Its easy to lose track of
> if a specific variable is supposed to be escaped between the template
> file and the call into TemplateProcessor.

Let's not go too easy on Mustache, there are several more reasons why
these templates are full of security gaps:

* Escaping or failing to escape HTML is the difference between {{ }} and
{{{ }}}, and unless you spent your whole life writing Mustache
templates, you won't remember which is which.

* Mustache has no concept of HTML structure, or any structure, or
variable types; it just concatenates strings, so it's difficult to
automatically detect any problems.


> Anyways, i'd like to propose a naming convention. Any mustache variable
> that is used as raw html should have some sort of easily identifiable
> prefix so it is easy to keep track of which parameters are escaped and
> which are not. e.g. instead of naming the parameter foo, it would be
> named something like HTMLFoo.

We already do this, at least! Most Mustache variables used as raw HTML
are prefixed with 'html-'. Vector is pretty consistent about this [1],
but even it has some exceptions. Other code is not all so good.

[1]
https://codesearch.wmcloud.org/search/?q={{{&files=\.mustache%24&excludeFiles=&repos=Skin%3AVector


--
Bartosz Dziewoński
_______________________________________________
Wikitech-l mailing list -- wikitech-l@lists.wikimedia.org
To unsubscribe send an email to wikitech-l-leave@lists.wikimedia.org
https://lists.wikimedia.org/postorius/lists/wikitech-l.lists.wikimedia.org/