thymeleaf / thymeleaf-spring

Thymeleaf integration module for Spring
http://www.thymeleaf.org
Apache License 2.0
435 stars 157 forks source link

Remote Code Execution Via SSTI #291

Closed wh0amitz closed 2 years ago

danielfernandez commented 2 years ago

Two things to note:

First if you think you have found a security vulnerability, it is hugely irresponsible publish it like this instead of first sending a message to the Thymeleaf Team at https://www.thymeleaf.org/team.html.

Second I cannot see how this is a security vulnerability in Thymeleaf itself. In your code, you are manually creating a template that includes, with no escaping at all, whatever is input as a request parameter directly into the template by means of String concatenation (variable name below):

    @ResponseBody
    @RequestMapping("/")
    String test(@RequestParam(required=false, name = "name") String name) throws Exception {
        ...
        String template ="<!DOCTYPE html><html><body>"+
                "<form action='/' method='post'>"+
                "First name:<br>"+
                "<input type='text' name='name' value=''>"+
                "<input type='submit' value='Submit'>"+
                "</form><h2>Hello "+
                name+                      // "name" is a request parameter that is simply concatenated in the middle of the template.
                "</h2></body></html>";
        ...
    }

Then you are manually creating a template engine object that executes string templates (i.e. not files):

        // StringTemplateResolver is here configured to make Thymeleaf execute the
        // template String created in the code above
        StringTemplateResolver resolver = new StringTemplateResolver();
        resolver.setTemplateMode(TemplateMode.HTML);

        SpringTemplateEngine engine = new SpringTemplateEngine();
        engine.setTemplateResolver(resolver);

And then, finally, you manually execute that template you just created and make your controller directly return it as response payload by means of adding @ResponseBody to the controller:

        StringWriter out = new StringWriter();
        Context context = new Context();
        engine.process(template, context, out);
        template = out.toString();

There is no way this can be considered a security vulnerability in Thymeleaf: it is a security hole created willingly by your code.

You are going to great lengths in order to force the template processor's way until you get it to execute code coming directly from a user form. This is a bit like willingly creating a controller that executes whichever SQL query you receive via JDBC and then call a security vulnerability in JDBC the fact that your code allows for the execution of arbitrary SQL queries.

The reason you are doing this in such a forced manner is because if you tried to use th:utext="${param.name}" (or the equivalent [(${param.name})]) for inserting that text unescaped into your template, which is the way Thymeleaf offers for inserting unescaped content into templates:

<!DOCTYPE html>
<html><body>
  <form action='/' method='post'>
    First name:<br>
    <input type='text' name='name' value=''>
    <input type='submit' value='Submit'>
  </form>
  <h2>Hello [(${param.name})]</h2>
</body></html>

Then Thymeleaf would detect that you are trying to use a request parameter (name) in a restricted environment, and that would provoke an exception.

But instead of that, you are forcing your way throught the template engine's security mechanisms until there is no way Thymeleaf can detect that a part of the template it is executing comes from an unsafe origin. How can you expect the template engine to know about the origin of each of the lines of the templates it is executing?

Of course, there will always be a way to make Thymeleaf ignore the origin of your code, your expressions and your data if your own application code tries hard enough to deceive it. But it's not realistic to expect the template engine to scan your entire application's code and warn you of your own wrongdoings.

I'm therefore dismissing this as unrealistic code and also unrealistic expectations.