Secure by default templates

Development
#1

Looking at recently published security advisories like CVE-2022-29251 it is apparent that it is very easy to introduce security vulnerabilities in XWiki as by default output from Velocity isn’t escaped in any way. This means that every time you output a value, you need to actively remember that it needs to be escaped - or you need to decide that this value is safe and doesn’t need escaping. Modern UI frameworks like React or Vue.js escape all values by default which is very effective at preventing security issues. I think we should include such secure-by-default templates for server-side rendering in XWiki and strongly encourage their use over direct output from Velocity. The main motivation for me is to make it easy to write secure code without requiring much thinking.

I would formulate the following requirements:

  1. Support for both XWiki syntax and HTML with similar syntax
  2. Support for the existing script context such that no new API needs to be created and variables (and maybe even macros) defined in Velocity can be accessed
  3. A syntax that is not executed by Velocity such that it can be used in Velocity macros and templates (would need extra post-processing, though, for templates, not sure if we want to do this)

There are existing frameworks like Thymeleaf in Java which support templates with parameters prefixed by th: that include automatic escaping of the used value:

<input type="text" name="userName" value="James Carrot" th:value="${user.name}" />

They also support the special text attribute to set the content of an element. This syntax is incompatible with Velocity so this would fail the third requirement but we should check how much this can be customized. For XWiki syntax I imagine something similar, using the existing parameter syntax:

(% xw:id="user.id" xw:text="user.name" %)
= User Name =

where “User Name” would be replaced by the user’s name. Similarly, this could also support parameters in macros and references.

This kind of parameter support typically also supports simple loops and if-statements such that simple display logic can be directly implemented in the template using special “if” and “each” attributes, see the Thymeleaf Tutorial for some examples.

While the creation of a special version of the HTML macro for such a parameter syntax should be relatively straightforward, the use in XWiki syntax creates some questions:

  • Should we support more parameter types than just simple strings for macros? This could be interesting to create macros that would, e.g., display an XObject that is passed as parameter.
  • In which order are these parameters parsed, in particular in combination with macros? As these parameters could reference values from velocity macros, they should be parsed after velocity macros, on the other if there is an if-parameter on some higher-level element, this should prevent parsing the velocity macro, so I think this would need to be mixed with macro execution in a clever way.
  • We would need a strategy how to transform these special parameters into valid HTML (data-)-attributes without parsing in the annotated HTML renderer such that they can be preserved in the WYSIWYG editor. In particular for macros, this might also be challenging and we might need to disable the execution of macros that require parameters in the context of annotated HTML.

All in all, this doesn’t seem easy and I’m wondering if it would be better to restrict this parameter syntax to syntax parsed in special macros, or in other words: just apply this parameter processing to the XDOM that is parsed from the content of a new macro.

I’ve also no idea when this could be implemented, this totally depends on the complexity (which is why I would prefer to keep this as simple as possible) and if somebody has the time to actually implement it. In the end I hope this will actually save time, though, as it should hopefully make the development easier as you need to think less about escaping and it will prevent security issues which create a huge cost later not only for fixing and documenting them but also in terms of potential harm they could cause. Therefore, I hope we can implement this as soon as possible such that new code can be written using the new parameter support and existing code can be migrated to use it.

Any opinions? Ideas? Any suggestions for frameworks we could use for parameter content parsing?

1 Like
#2

+1 for the general idea

+1 for those requirements but for 2) I would talk about ScriptContext in general and not limit it to Velocity variables

I think the support for HTML and XWiki syntax sources should be two separated subjects as they have very different constraints and limitations and one should not impact the other in more details that following the same general concept.

#3

I read quickly and I’m not sure I understood everything. Let me try to rephrase to see what I understood (or not ;)).

It seems that you’re proposing to add support for a new templating engine in XWiki so that dynamic content can be inserted inside xwiki/2.x syntax (your example is showing this for a parameter on a HeaderBlock).

Note: Supporting different template engines (and not just Velocity) has been an idea from a long time ago (we had a GSOC proposal for FreeMarker if I recall correctly and we mentioned Thymeleaf too in the past).

So if I understand correctly, you’re proposing to drop (or not recommend) the usage of {{velocity}}...{{html}}... and instead, when generating HTML content (and XWiki Syntax), provide another mechanism to insert dynamic values. So, for the {{html}} macro, something like:

{{html}}
...
<input type="hidden" name="newThemeName" id="newThemeName" th:value="request.newThemeName" />

For me, this really looks like adding support for a new template engine in XWiki, in addition to Velocity.

I don’t see a problem of introducing another templating engine, that would be activated in the content of a macro, say a {{thymeleaf}} macro for example. That allows to have both {{velocity}} and {{thymleaf}} in a document’s content. That would use the existing concept of macros.

I’d say yes, since this is supported in the Java implementation of the macro and all we’re doing is converting a string to the target java type.

Thanks

#4

Yes, that’s basically the idea, with the goal that this template engine allows to safely inject parameters/attributes into XWiki syntax and HTML and doesn’t require the manual escaping that is so easy to get wrong.

The idea would be that you would still need to use Velocity for processing user input, for example, but what I would like to deprecate is to produce wiki syntax output (including nested macros like the HTML macro) with Velocity and use whatever new template engine we add for this. I understand that this is quite contrary to the original purpose of Velocity, but in XWiki we’re in fact using Velocity as a general purpose scripting language and while I don’t like it, I don’t think it’s realistic to replace it anytime soon.

Support for HTML and XWiki syntax can be two completely different subjects with different constraints and limitations, as pointed out by @tmortagne. Still, I would try to keep them very similar such that developers don’t need to learn two different syntaxes. I think we should look for a template engine that supports HTML but is modular and flexible enough to be usable in an XDOM transformation such that we can get the same loop/condition/variable/… support for both HTML and XWiki syntax.

#5

I’ve changed my opinion about adding script parameters to XWiki syntax. I think we’re relying in far too many places on the fact that anything that doesn’t contain a macro isn’t a script and thus reasonably safe (e.g., to avoid script in an HTML macro with wiki="true", it wouldn’t be sufficient anymore to call $escaptool.xml()). Therefore, I think as a start we should only implement this kind of dynamic parameter support for XWiki syntax in a new macro that will be treated like a script macro. Dynamic parameters would then also only work on macros that are direct children of that script macro and not on content that is further nested in macros. The main difference to the current Velocity macro would be parsing order: instead of first parsing Velocity and then XWiki syntax, we would be first parsing XWiki syntax and then evaluate the scripts in the parameters in an XDOM transformation on the macro content.

Once we’ve gained some experience with this, maybe found better ways how to track authorship in the XDOM and migrated content to no longer rely on printing HTML/XWiki syntax output in Velocity, we can consider supporting dynamic parameters globally in a future version of XWiki syntax.

#6

Sounds good. This is what I had in mind when I replied above.

#7

Coming back to this almost half a year later, I still think we absolute must implement this and I want to add here some more recent thoughts how to implement this. What I imagine are two macros with the following behavior:

  • The content of the macro is parsed as HTML in the one macro and as XWiki syntax in the other. The result of both is an XDOM. We should develop a new HTML parser for this and introduce a new block to represent all HTML elements for which we don’t have dedicated blocks yet (alternative: introduce dedicated blocks).
  • A transformation is applied on the XDOM that executes scripts that are found in parameters starting with xw: and either replaces them by new parameters without the prefix or executes special actions for:
    • for loops (duplicates blocks)
    • if/elseif/else (removes blocks conditionally)
    • content: parses the result as plain text and adds it as child blocks except for macros, where it would be added as a string. With a special prefix it should also be possible to treat the provided content as raw input for a specified syntax. Further, it should also be possible that the script for the content returns XDOM blocks that are then directly inserted as children.
    • Special handling for macro blocks where parameters are directly converted to the parameter type (e.g., directly pass a list of strings to the parameters of the translation macro, a UIX could be passed as object to a future version of some UIX macro).
  • The result should be a regular XDOM that can be further transformed and then rendered into any output syntax.

Ideally, we would use some existing framework for the script execution part, I haven’t had any look yet what Thymeleaf or Velocity provide here. The script part should also use the existing scripting API, script scope etc. such that, e.g., a Velocity macro can process a request and prepare some variables and then the new template macros can be used to display the result.

At the moment, many wiki pages use helper Velocity macros for generating parts of the content. My idea would be to transform these Velocity macros into XWiki macros. We could introduce support for custom HTML elements in the HTML parser like <xwiki-macro macro-name="translation" macro-parameter-key="some.translation"> to support using macros even in HTML content similar to how you can use, e.g., other Vue.js components in Vue.js. I’m wondering here if it would make sense to introduce support for “on-demand” macros that are only available in a document once they have been explicitly included/imported in some way (to be determined) to avoid cluttering the global macro namespace.