thymeleaf / thymeleaf-docs

Thymeleaf documentation
Apache License 2.0
44 stars 54 forks source link

XSS vulnerability in "4.1 Messages" example code #62

Closed ghost closed 6 years ago

ghost commented 6 years ago

There is a cross-site scripting vulnerability in the 3.0 Using Thymeleaf tutorial's first example of a parameterized message, at https://www.thymeleaf.org/doc/tutorials/3.0/usingthymeleaf.html#messages

<p th:utext="#{home.welcome(${session.user.name})}">
  Welcome to our grocery store, Sebastian Pepper!
</p>

.. where home.welcome is:

home.welcome=¡Bienvenido a nuestra tienda de comestibles, {0}!

Because the formatted output is used for th:utext, if session.user.name contains HTML, it is included verbatim within the generated paragraph element's content.

Tested: Thymeleaf 3.0.9

danielfernandez commented 6 years ago

I'm honestly not convinced that could be called an XSS vulnerability. If that were true, actually every th:utext use or any other type of unescaped output would be, too. Thymeleaf is the view layer of an application, and as such it cannot have the responsibility of checking the values of the request/session attributes (model attributes in Spring jargon) that the developer's code creates before the view's execution. A session attribute is not direct user input, it is something that has gone through (at least) controller validation code. That's precisely one of the responsibilities of the controller layer (including security filters). And it's the controller (or a filter) who has willingly put that user variable into the session as an attribute.

It would be different if that code was directly using a request parameter (not attribute), like:

<p th:utext="#{home.welcome(${param.username})}">
  Welcome to our grocery store, Sebastian Pepper!
</p>

In such case, Thymeleaf would be allowing direct, unfiltered use of user input (in this case, a request parameter) in an unescaped environment. It would not be giving the controller the opportunity to adequately filter and validate user input. But this is precisely something that Thymeleaf does not allow — the above code throws an error.

ghost commented 6 years ago

You are right, of course, that th:utext is working as designed here, and that it is the application's responsibility to ensure that any arguments to a parameterized message used for th:utext are properly escaped.

I am assuming that something like session.user.name is not HTML-escaped. I think this is a fair assumption for typical web applications. session.user might be a POJO storing information from a users table for the currently-logged in user. In such a scenario, the name field would be the user's selected name, which is probably controllable by the user. An attacker could set their name to something containing malicious HTML. When printing session.user.name, this would likely affect only the attacker, but if other users' names are printed (say post.user.name), then the malicious HTML would be included in pages viewed by victim users.

Would something like the following be the recommended approach for escaping arguments to parameterized messages:

<p th:utext="#{home.welcome( ${#strings.escapeXml(session.user.name)} )}">
  Welcome to our grocery store, Sebastian Pepper!
</p>

Maybe this would be good to include in the tutorial?

danielfernandez commented 6 years ago

Thanks @dtrebbien!