Issue ID: CVE-2021-32827

[bhmohanr-techie]

Issue ID:

CVE-2021-32827

Issue Description:

MockServer is open source software which enables easy mocking of any system you integrate with via HTTP or HTTPS. An attacker that can trick a victim into visiting a malicious site while running MockServer locally, will be able to run arbitrary code on the MockServer machine. With an overly broad default CORS configuration MockServer allows any site to send cross-site requests. Additionally, MockServer allows you to create dynamic expectations using Javascript or Velocity templates. Both engines may allow an attacker to execute arbitrary code on-behalf of MockServer. By combining these two issues (Overly broad CORS configuration + Script injection), an attacker could serve a malicious page so that if a developer running MockServer visits it, they will get compromised. For more details including a PoC see the referenced GHSL-2021-059

Affected versions:

5.13.2 and earlier

Vulnerability Type:

Remote Code Execution (RCE)

Fix:

Please note that, the fix added here is via properties (enabling these properties, will address the issue). This will ensure that we are not breaking functionality for existing users.

Java Script Template:

• The issue reported here is, the java code is injected in javascript engine with a statement like: this.engine.factory.scriptEngine.eval(). Ex: this.engine.factory.scriptEngine.eval(‘java.lang.Runtime.getRuntime(“calc.exe”)’).
• Here there are two points to be noted: 1. it allows triggering java classes that can exploit the target machine, 2. the engine object is exposed to the users via javascript
• For point 1, where it allows triggering a malicious code (like java.lang.Runtime in this case), We can mitigate this vulnerability in javascripts, by implementing ClassFilter interface provided by JDK. This interface contains a method exposeToScripts. By overriding this method, we can prevent dangerous methods from being called via reflection API. Added changes in JavaScriptTemplateEngine to create a new class filter. Here a new property “mockserver.javascript.class.deny” is added, which contains a list of java classes to be restricted (comma separated), if a javascript triggers these classes, the engine prevents the same.
• For point 2, where engine object is exposed. The fix required here is, we need to block engine object getting exposed. The issue here is, the statement this.engine.factory.scriptEngine.eval breaks down to:

• this.engine --> global engine,
• factory --> getFactory(), returns NashornScriptEngineFactory instance,
• scriptEngine --> getScriptEngine() creates a fresh engine instance with default (== no ClassFilter, etc..) configuration,
• and finally .eval([script]), execute JavaScript code with full access to the Java world.

• So, the fix here should be to prevent exposing engine and factory objects, thereby preventing user to create his own new engine instances. In order to do this, adding a new property “mockserver.javascript.text.deny” which contains a list of backlisted strings (comma separated), that will result in preventing the script getting triggered.

• In addition to fixing the issue reported, the newly added properties also help in fixing few other cases: where a malicious java code is triggered with existing engine, rather than creating a new one.

Velocity Template:

• The issue reported here is, the java code is injected in velocity engine with a statement like: $!request.class.forName('java.lang.Runtime').getRuntime().exec(encodeURIComponent('notepad.exe'))
• Here the issue is due to vulnerability in velocity-engine-core, which is addressed in 2.3 version. Here, the velocity code provides a uberspector implementation: SecureUberspector, which says: “Use a custom introspector that prevents classloader related method calls. Use this introspector for situations in which template writers are numerous or untrusted. Specifically, this introspector prevents creation of arbitrary objects or reflection on objects.”
• Mockserver already uses 2.3 version of velocity-engine-core, however there is no uberspector configured.
• Added changes here to configure the default apache velocity uberspector, in VelocityTemplateEngine. This change again is enabled when property mockserver.velocity.class.deny=true is set.

Workarounds:

For Javascript templates

• Launching mockserver with an additional nashorn argument “—no-java” will prevent exposing any java code to java scripts. But this will result in preventing any java code.
• If mockserver is triggered with two things: a security manager and a class filter. This will prevent exposing the engine object, thereby preventing any java code from getting triggered. Again, this will prevent any java code.

For Velocity templates

• No workaround available

Unit Testing:

• Verified that the RCE vulnerability is no more applicable for "JavaScript" and "Velocity" templates.
• Verified the fix in Chrome, Mozilla and Firefox browsers.
• All the test cases for this change is covered in unit test code: in JavaScriptTemplateEngineTest.java and VelocityTemplateEngineTest.java

Summary:

• The changes added in the pull request, takes effect only if the newly added properties are set. This ensures that existing users of mockserver are not affected by this change.
• The new properties can be configured based on the environment and user needs.
• In order to fix CVE-2021-32827, we need to restrict two things: 1. accessing java.lang.Runtime and 2. accessing engine object.
• Fix for JavaScript templates:
  mockserver.javascript.text.deny=engine.factory and mockserver.javascript.class.deny=java.lang.Runtime
• Fix for Velocity templates:
  mockserver.velocity.class.deny=java.lang.Runtime

[bhmohanr-techie]

For more details on the issue and the changes added, please refer: Mockserver - Triage Document - CVE-2021-32827.pdf

[jamesdbloom]

@bhmohanr-techie I have reviewed your code and the basic approach looks good but I have noticed the following issues:

• there are no tests that I can see that actually test the logic being enabled
• the test assertions have if blocks around them, this is an error prone way to write tests as it is not clear if the assertion are hit, also the test is only run one way so only one branch will be hit for the if-else meaning it adds no value
• the configuration should be propagated into the Configuration class, this ensure multiple instances of MockServer in the same JVM can be configured differently

I'm happy to merge the code and fix these points myself or let you do it as you prefer?

[bhmohanr-techie]

@bhmohanr-techie I have reviewed your code and the basic approach looks good but I have noticed the following issues:

• there are no tests that I can see that actually test the logic being enabled
• the test assertions have if blocks around them, this is an error prone way to write tests as it is not clear if the assertion are hit, also the test is only run one way so only one branch will be hit for the if-else meaning it adds no value
• the configuration should be propagated into the Configuration class, this ensure multiple instances of MockServer in the same JVM can be configured differently

I'm happy to merge the code and fix these points myself or let you do it as you prefer?

Hi @jamesdbloom ,

Thankyou for your review comments.

I have added changes to address all the comments you have raised, and committed the changes.

Below are the changes added,

• So far, the properties were accessed via ConfigurationProperties class, here changes are added to access properties via Configuration class
• Now, the testcases for javascript templates are separated into individual unit test methods (around 12 methods, covering all possible cases). In each of these unit test methods, the properties are configured and the actual logic gets evaluated
• Now there are 13 testcases for the changes added in this PR, and I have verified that all these tests are passed.
• I have also added a comment in each unit test method, which gives a brief about the testcase being evaluated in the method and the expected result. For example,
  public void shouldHandleHttpRequestsWithJavaScriptTemplateWithJavaStringsViaNewEngineWithDeniedClassWithDeniedText() {
  // Test Case: mockserver.javascript.class.deny=java.lang.Runtime, and mockserver.javascript.text.deny=engine.factory
  // Expected Result: Deny javascript execution due to restricted text found in template
  ......
  ......
  }

Please review and let me know, if you have any further suggestions.

Thanks.

[jamesdbloom]

Thanks @bhmohanr-techie for the improvements this looks good.