Skip to content

Commit

Permalink
Fix #1855
Browse files Browse the repository at this point in the history
  • Loading branch information
cowtowncoder committed Dec 19, 2017
1 parent f031f27 commit 2235894
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 49 deletions.
Expand Up @@ -12,6 +12,7 @@
import com.fasterxml.jackson.databind.deser.std.ThrowableDeserializer;
import com.fasterxml.jackson.databind.introspect.*;
import com.fasterxml.jackson.databind.jsontype.TypeDeserializer;
import com.fasterxml.jackson.databind.jsontype.impl.SubTypeValidator;
import com.fasterxml.jackson.databind.util.ArrayBuilders;
import com.fasterxml.jackson.databind.util.ClassUtil;
import com.fasterxml.jackson.databind.util.SimpleBeanPropertyDefinition;
Expand Down Expand Up @@ -40,44 +41,6 @@ public class BeanDeserializerFactory

private final static Class<?>[] NO_VIEWS = new Class<?>[0];

/**
* Set of well-known "nasty classes", deserialization of which is considered dangerous
* and should (and is) prevented by default.
*/
protected final static Set<String> DEFAULT_NO_DESER_CLASS_NAMES;
static {
Set<String> s = new HashSet<String>();
// Courtesy of [https://github.com/kantega/notsoserial]:
// (and wrt [databind#1599])
s.add("org.apache.commons.collections.functors.InvokerTransformer");
s.add("org.apache.commons.collections.functors.InstantiateTransformer");
s.add("org.apache.commons.collections4.functors.InvokerTransformer");
s.add("org.apache.commons.collections4.functors.InstantiateTransformer");
s.add("org.codehaus.groovy.runtime.ConvertedClosure");
s.add("org.codehaus.groovy.runtime.MethodClosure");
s.add("org.springframework.beans.factory.ObjectFactory");
s.add("com.sun.org.apache.xalan.internal.xsltc.trax.TemplatesImpl");
s.add("org.apache.xalan.xsltc.trax.TemplatesImpl");
// [databind#1680]: may or may not be problem, take no chance
s.add("com.sun.rowset.JdbcRowSetImpl");
// [databind#1737]; JDK provided
s.add("java.util.logging.FileHandler");
s.add("java.rmi.server.UnicastRemoteObject");
// [databind#1737]; 3rd party
s.add("org.springframework.aop.support.AbstractBeanFactoryPointcutAdvisor");
s.add("org.springframework.beans.factory.config.PropertyPathFactoryBean");
s.add("com.mchange.v2.c3p0.JndiRefForwardingDataSource");
s.add("com.mchange.v2.c3p0.WrapperConnectionPoolDataSource");
// [databind#1855]: more 3rd party
s.add("org.apache.tomcat.dbcp.dbcp2.BasicDataSource");
s.add("com.sun.org.apache.bcel.internal.util.ClassLoader");
DEFAULT_NO_DESER_CLASS_NAMES = Collections.unmodifiableSet(s);
}

/**
* Set of class names of types that are never to be deserialized.
*/
protected Set<String> _cfgIllegalClassNames = DEFAULT_NO_DESER_CLASS_NAMES;

/*
/**********************************************************
Expand Down Expand Up @@ -179,7 +142,7 @@ public JsonDeserializer<Object> createBeanDeserializer(DeserializationContext ct
return null;
}
// For checks like [databind#1599]
checkIllegalTypes(ctxt, type, beanDesc);
_validateSubType(ctxt, type, beanDesc);
// Use generic bean introspection to build deserializer
return buildBeanDeserializer(ctxt, type, beanDesc);
}
Expand Down Expand Up @@ -877,19 +840,12 @@ protected boolean isIgnorableType(DeserializationConfig config, BeanDescription
}

/**
* @since 2.8.9
* @since 2.8.11
*/
protected void checkIllegalTypes(DeserializationContext ctxt, JavaType type,
protected void _validateSubType(DeserializationContext ctxt, JavaType type,
BeanDescription beanDesc)
throws JsonMappingException
{
// There are certain nasty classes that could cause problems, mostly
// via default typing -- catch them here.
String full = type.getRawClass().getName();

if (_cfgIllegalClassNames.contains(full)) {
throw JsonMappingException.from(ctxt,
String.format("Illegal type (%s) to deserialize: prevented for security reasons", full));
}
SubTypeValidator.instance().validateSubType(ctxt, type);
}
}
@@ -0,0 +1,98 @@
package com.fasterxml.jackson.databind.jsontype.impl;

import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.JsonMappingException;

/**
* Helper class used to encapsulate rules that determine subtypes that
* are invalid to use, even with default typing, mostly due to security
* concerns.
* Used by <code>BeanDeserializerFacotry</code>
*
* @since 2.8.11
*/
public class SubTypeValidator
{
protected final static String PREFIX_STRING = "org.springframework.";
/**
* Set of well-known "nasty classes", deserialization of which is considered dangerous
* and should (and is) prevented by default.
*/
protected final static Set<String> DEFAULT_NO_DESER_CLASS_NAMES;
static {
Set<String> s = new HashSet<String>();
// Courtesy of [https://github.com/kantega/notsoserial]:
// (and wrt [databind#1599])
s.add("org.apache.commons.collections.functors.InvokerTransformer");
s.add("org.apache.commons.collections.functors.InstantiateTransformer");
s.add("org.apache.commons.collections4.functors.InvokerTransformer");
s.add("org.apache.commons.collections4.functors.InstantiateTransformer");
s.add("org.codehaus.groovy.runtime.ConvertedClosure");
s.add("org.codehaus.groovy.runtime.MethodClosure");
s.add("org.springframework.beans.factory.ObjectFactory");
s.add("com.sun.org.apache.xalan.internal.xsltc.trax.TemplatesImpl");
s.add("org.apache.xalan.xsltc.trax.TemplatesImpl");
// [databind#1680]: may or may not be problem, take no chance
s.add("com.sun.rowset.JdbcRowSetImpl");
// [databind#1737]; JDK provided
s.add("java.util.logging.FileHandler");
s.add("java.rmi.server.UnicastRemoteObject");
// [databind#1737]; 3rd party
//s.add("org.springframework.aop.support.AbstractBeanFactoryPointcutAdvisor"); // deprecated by [databind#1855]
s.add("org.springframework.beans.factory.config.PropertyPathFactoryBean");
s.add("com.mchange.v2.c3p0.JndiRefForwardingDataSource");
s.add("com.mchange.v2.c3p0.WrapperConnectionPoolDataSource");
// [databind#1855]: more 3rd party
s.add("org.apache.tomcat.dbcp.dbcp2.BasicDataSource");
s.add("com.sun.org.apache.bcel.internal.util.ClassLoader");
DEFAULT_NO_DESER_CLASS_NAMES = Collections.unmodifiableSet(s);
}

/**
* Set of class names of types that are never to be deserialized.
*/
protected Set<String> _cfgIllegalClassNames = DEFAULT_NO_DESER_CLASS_NAMES;

private final static SubTypeValidator instance = new SubTypeValidator();

protected SubTypeValidator() { }

public static SubTypeValidator instance() { return instance; }

public void validateSubType(DeserializationContext ctxt, JavaType type) throws JsonMappingException
{
// There are certain nasty classes that could cause problems, mostly
// via default typing -- catch them here.
final Class<?> raw = type.getRawClass();
String full = raw.getName();

do {
if (_cfgIllegalClassNames.contains(full)) {
break;
}

// 18-Dec-2017, tatu: As per [databind#1855], need bit more sophisticated handling
// for some Spring framework types
if (full.startsWith(PREFIX_STRING)) {
for (Class<?> cls = raw; cls != Object.class; cls = cls.getSuperclass()) {
String name = cls.getSimpleName();
// looking for "AbstractBeanFactoryPointcutAdvisor" but no point to allow any is there?
if ("AbstractPointcutAdvisor".equals(name)
// ditto for "FileSystemXmlApplicationContext": block all ApplicationContexts
|| "AbstractApplicationContext.equals".equals(name)) {

This comment has been minimized.

Copy link
@gaol

gaol Dec 25, 2017

Are you sure this is not a typo?

This comment has been minimized.

Copy link
@cowtowncoder

cowtowncoder Dec 26, 2017

Author Member

Yes; it was fixed in a later patch. Thank you for pointing it out, still, as somehow I managed not to read my own code before commit -- and if that hadn't been changed, fix obv wouldnt have worked.

This comment has been minimized.

Copy link
@gaol

gaol Dec 27, 2017

Cool, I got this link from one Bugzilla issue which says this commit fixed the problem. I should look more commits though. Thank you very much for the explanation, and happy new year 😄

break;

This comment has been minimized.

Copy link
@gaol

gaol Dec 25, 2017

Shouldn't this break the outer loop?

This comment has been minimized.

Copy link
@cowtowncoder

cowtowncoder Dec 26, 2017

Author Member

Same as above, yes, absolutely needs to use label. Was fixed on a later commit.

}
}
}
return;
} while (false);

throw JsonMappingException.from(ctxt,
String.format("Illegal type (%s) to deserialize: prevented for security reasons", full));
}
}

0 comments on commit 2235894

Please sign in to comment.