Closed harwalan closed 2 years ago
If understand correctly, in this case the "Remote" in RCE would come from the fact that the users of your application can actually develop Thymeleaf templates by themselves using the Standard dialects, and then… upload them to your server for them to be executed?
If this is correct, I would definitely say that the vulnerability in this case is in your application and not in Thymeleaf.
Given the type of templates Thymeleaf is meant to serve when using the Standard dialects, which include generic expressions like the ones in Spring EL, I think the scenario you describe is a bit like allowing your users to upload .class
files to be loaded and executed by the JDK, and then calling that an RCE in the JVM.
Several other template engines allow the inclusion of Java code in them, easiest example would be JSP itself. But I don't think considering this fact an RCE vulnerability in JSP would be correct.
In my opinion, a functionality such as the one you describe in your application would require a different kind of templates — certainly ones that are much more restrictive. You can either do this in Thymeleaf itself by implementing your own dialect –which can be as restrictive as you want– and then allow your users to develop templates that use only this dialect... or you can alternatively use a simpler, more restrictive template engine instead of Thymeleaf.
Here we have to differentiate your two expressions because they are two very different cases. This is the first one:
th:text="${''.getClass().forName('java' + '.lang' + '.Runtime')}"
This works simply because you are obtaining the class reference for java.lang.Runtime
, but you are not doing anything with it. The java.lang.Runtime
class is blocked, so any attempts to call methods on it will throw an exception. But you are not calling any methods, so this is harmless.
Now for the second expression:
th:text="${''.getClass().forName('javax.script.ScriptEngineManager').newInstance()...}"
This is allowed simply because javax.script.ScriptEngineManager
is not a blocked class. I would agree that it should be, given java.lang.Runtime
is, and I can confirm that it will be added in the next version. But still, considering this a vulnerability would miss the point on what Thymeleaf protects from.
It would not be reasonable to expect Thymeleaf to protect from its own template developers, or expect it to contemplate the possibility that templates themselves could be inserted from the outside without validation.
What Thymeleaf protects from is direct, unfiltered user input, like request parameters. And for this it implements a restricted mode.
Thymeleaf's restricted execution mode is applied in general wherever a template might be processing direct, unfiltered user input, such as request parameters. This include pre-processing expressions (__...__
), unescaped output (th:utext
) and inlined unescaped expressions, links, event handlers, fragment expressions…
But this is not one of those scenarios: first, because it is using th:text
, which is meant to simply output escaped text; and second and most important, because there is no direct user input involved in this template expression. It is an expression that has been included in the template by the developer him/herself.
So in this case, what you are bypassing with this expression using javax.script.ScriptEngineManager
is not really the main check, which Thymeleaf imposes on direct user input, but an additional restriction introduced in 3.0.15 itself "for higher security": the blocked classes (see thymeleaf/thymeleaf#871). Thymeleaf 3.1 (already in development) will implement a much larger control over what classes can be used in expressions, but Thymeleaf 3.0 simply implements this with a list of blocked classes. After all, this is a secondary check for the sake of peace of mind, and the aim keeps being exclusively to control user input.
And again, a th:text
with a value like yours is not user input, and therefore only the secondary, additional check applies.
\ So my conclusion here is that, for this specific scenario, Thymeleaf standard templates are not the right choice. I would definitely recommend using a custom dialect, or alternatively a different technology.
\ Also, please note that publishing about possible vulnerabilities in public forums before contacting the project team and discussing the scenario in private creates unnecessary alarm and is, in general, a very bad practice.
@danielfernandez I really appreciate your thorough response! I also apologize that I used a less preferred method for communicating potential vulnerabilities. I totally see the reasons behind it.
I'm already invested in ThymeLeaf and would love to find a way to close off the evaluation of Java inside a template. Is there a way to do this? Thanks!
I'm already invested in ThymeLeaf and would love to find a way to close off the evaluation of Java inside a template. Is there a way to do this? Thanks!
You mean that you would like to be able to execute some Java in your templates, but without all what the Standard dialects offer? If so, this would require creating a custom dialect and adding Java execution capabilities to it, perhaps through some expression language (i.e. not SpringEL) that allows for a restricted use of Java code... or else use SpringEL but restrict quite a lot its Java capabilities...
I am able to produce remote code execution that appears to be enabled by ThymeLeaf version 3.0.15 using spring. It looks kind of like this, but perhaps it helps if I explain the setup.
My app allows the user to customize their own templates. If a user puts java code in their template the Java is executed. Here's an example:
<div th:text="${''.getClass().forName('java' + '.lang' + '.Runtime')}"></div> .
another example:
<div th:text="${''.getClass().forName('javax.script.ScriptEngineManager').newInstance().getEngineByName('JavaScript').eval('var p = new java.lang.ProcessBuilder();p.command("bash", "-c", "id"); p.redirectErrorStream(true); var process=p.start(); var inputStreamReader = new java.io.DataInputStream(process.getInputStream());output=inputStreamReader.readLine();')} "></div> .
In the other case (referenced above), this comment was made:
Does this comment assume that only a developer has access to the templates? If yes, I'd like to eliminate the assumption and ask that this be considered a high priority vulnerability.
Thanks!