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=%7B%7B%7B&files=%5C.mustache%24...