Bug 65151 - Add a sandbox for JEXL
Summary: Add a sandbox for JEXL
Status: NEW
Alias: None
Product: JMeter
Classification: Unclassified
Component: Main (show other bugs)
Version: unspecified
Hardware: All All
: P2 enhancement (vote)
Target Milestone: JMETER 5.4.1
Assignee: JMeter issues mailing list
Depends on:
Reported: 2021-02-20 20:47 UTC by Artem
Modified: 2021-02-20 20:48 UTC (History)
1 user (show)

Test plan with a JEXL expression that starts a new process (8.45 KB, application/xml)
2021-02-20 20:47 UTC, Artem
Patch (13.28 KB, patch)
2021-02-20 20:48 UTC, Artem
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Artem 2021-02-20 20:47:56 UTC
Created attachment 37738 [details]
Test plan with a JEXL expression that starts a new process

(after discussing this on security@apache.org, it was agreed to treat it as a potential security enhancement - below is the original report)

Apache JMeter offers __jexl2() and __jexl3() functions that evaluate JEXL expressions using Apache Commons JEXL. By default, JexlEngine evaluates in a powerful context that allows accessing all classes available in the JVM. As a result, it may let a malicious user run arbitrary code in a victim's machine. An attack scenario looks like the following:

1. An attacker prepares a malicious test plan that contains a call to one of the JEXL functions with a malicious payload.
2. The attacker tricks a victim to open and run the malicious test plan.
3. The payload runs on the victim's machine.

If the attacker is able to modify a victim's test plan, that may lead to arbitrary code execution. As an example, I am attaching a test plan with a JEXL expression that starts a new process. I tested it on Linux to run gedit. In other environments, you may need to replace gedit with some other application (for example, calc.exe).

Unfortunately, I can't tell if those are the only possible scenarios since I have not worked with JMeter before. It would be good if someone who has more experience with JMeter checked for other possible ways how a dangerous JEXL expression can be run via __jexl2() and __jexl3() functions.

The attack scenarios above may not be easy to implement since they require user iterations. In the second scenario, the attacker also needs to have access to the victim's test plans. That definitely decreases the likelihood of a successful attack to low-medium. Since a successful attack leads to arbitrary code execution, the impact looks pretty high. Therefore, the overall risk may be assigned to medium-high.

I am attaching a draft patch that introduces a sandbox for JEXL expressions. The sandbox is configured by several properties:
- "jexl.allowed.classes" is a list of comma-separated patterns that tells which methods JEXL functions are allowed to call
- "jexl.disallowed.classes" is a list of comma-separated patterns that tells which methods JEXL functions are not allowed to call
- "jexl.allow.by.default" is a flag that tells JMeter to block access to methods by default

The solution implements a restrictive mode. It blocks all method calls except the ones that are explicitly allowed. The default allowed list contains classes from the org.apache.jmeter and several common classes from the java.lang package. I believe this draft list may be extended with more useful and safe classes. That's a restrictive strategy that makes JEXL evaluation safer by default. Even if the list contains many classes, such a restrictive strategy introduces a risk that the update breaks user's test plans. To fix them, they'll need to add the classes they use to the list. If we trade security for usability, we can allow all classes by default (jexl.allowed.classes = false) and update the docs with a note about possible attacks and how users can mitigate them.

Someone can say that is not an issue at all because a user is supposed to open and run only trusted test plans. In general, it's fine to have such a threat model. If so, it would be good to add a note in the docs about opening test plans from untrusted sources. In my opinion, it would be better to consider more threats and make JMeter safer by default. I may miss something though since I am not an expert in JMeter.
Comment 1 Artem 2021-02-20 20:48:41 UTC
Created attachment 37739 [details]