Skip to content

Commit

Permalink
Fix #1737 on 2.6 (#1945)
Browse files Browse the repository at this point in the history
  • Loading branch information
derekstraka authored and cowtowncoder committed Feb 26, 2018
1 parent 7da1b44 commit a054585
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 6 deletions.
3 changes: 3 additions & 0 deletions release-notes/VERSION
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ Project: jackson-databind
=== Releases ===
------------------------------------------------------------------------

2.6.7.2 (not yet released)
#1737: Block more JDK types from polymorphic deserialization

2.6.7.1 (11-Jul-2017)

#1383: Problem with `@JsonCreator` with 1-arg factory-method, implicit param names
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public class BeanDeserializerFactory
static {
Set<String> s = new HashSet<String>();
// Courtesy of [https://github.com/kantega/notsoserial]:
// (and wrt [databind#1599]
// (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");
Expand All @@ -58,6 +58,16 @@ public class BeanDeserializerFactory
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");

// [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");

DEFAULT_NO_DESER_CLASS_NAMES = Collections.unmodifiableSet(s);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.fasterxml.jackson.databind.interop;

import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.databind.*;

/**
Expand All @@ -13,11 +14,28 @@ static class Bean1599 {
public Object obj;
}

public void testIssue1599() throws Exception
static class PolyWrapper {
@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS,
include = JsonTypeInfo.As.WRAPPER_ARRAY)
public Object v;
}

/*
/**********************************************************
/* Unit tests
/**********************************************************
*/

private final ObjectMapper MAPPER = objectMapper();

// // // Tests for [databind#1599]

public void testXalanTypes1599() throws Exception
{
final String clsName = "com.sun.org.apache.xalan.internal.xsltc.trax.TemplatesImpl";
final String JSON = aposToQuotes(
"{'id': 124,\n"
+" 'obj':[ 'com.sun.org.apache.xalan.internal.xsltc.trax.TemplatesImpl',\n"
+" 'obj':[ '"+clsName+"',\n"
+" {\n"
+" 'transletBytecodes' : [ 'AAIAZQ==' ],\n"
+" 'transletName' : 'a.b',\n"
Expand All @@ -32,9 +50,75 @@ public void testIssue1599() throws Exception
mapper.readValue(JSON, Bean1599.class);
fail("Should not pass");
} catch (JsonMappingException e) {
verifyException(e, "Illegal type");
verifyException(e, "to deserialize");
verifyException(e, "prevented for security reasons");
_verifySecurityException(e, clsName);
}
}

// // // Tests for [databind#1737]

public void testJDKTypes1737() throws Exception
{
_testTypes1737(java.util.logging.FileHandler.class);
_testTypes1737(java.rmi.server.UnicastRemoteObject.class);
}

// 17-Aug-2017, tatu: Ideally would test handling of 3rd party types, too,
// but would require adding dependencies. This may be practical when
// checking done by module, but for now let's not do that for databind.

/*
public void testSpringTypes1737() throws Exception
{
_testTypes1737("org.springframework.aop.support.AbstractBeanFactoryPointcutAdvisor");
_testTypes1737("org.springframework.beans.factory.config.PropertyPathFactoryBean");
}
public void testC3P0Types1737() throws Exception
{
_testTypes1737("com.mchange.v2.c3p0.JndiRefForwardingDataSource");
_testTypes1737("com.mchange.v2.c3p0.WrapperConnectionPoolDataSource");
}
*/

private void _testTypes1737(Class<?> nasty) throws Exception {
_testTypes1737(nasty.getName());
}

private void _testTypes1737(String clsName) throws Exception
{
// While usually exploited via default typing let's not require
// it here; mechanism still the same
String json = aposToQuotes(
"{'v':['"+clsName+"','/tmp/foobar.txt']}"
);
try {
MAPPER.readValue(json, PolyWrapper.class);
fail("Should not pass");
} catch (JsonMappingException e) {
_verifySecurityException(e, clsName);
}
}

protected void _verifySecurityException(Throwable t, String clsName) throws Exception
{
// 17-Aug-2017, tatu: Expected type more granular in 2.9 (over 2.8)
_verifyException(t, JsonMappingException.class,
"Illegal type",
"to deserialize",
"prevented for security reasons");
verifyException(t, clsName);
}

protected void _verifyException(Throwable t, Class<?> expExcType,
String... patterns) throws Exception
{
Class<?> actExc = t.getClass();
if (!expExcType.isAssignableFrom(actExc)) {
fail("Expected Exception of type '"+expExcType.getName()+"', got '"
+actExc.getName()+"', message: "+t.getMessage());
}
for (String pattern : patterns) {
verifyException(t, pattern);
}
}
}

5 comments on commit a054585

@hosamaly
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @cowtowncoder. Can we expect this fix to be released soon?

@cowtowncoder
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have plans for releasing 2.6 patches.

But if there was someone within community who'd like to become owner/maintainer for older, closed branches, there is a possibility. At this point I do not have time to do that work unfortunately, so if there is need for such version this should be discussed on jackson-dev. If so, maintainer could merge important fixes; as is, 2.6 is probably missing many.

@hosamaly
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your response, @cowtowncoder. I thought that you mentioned that you intended to release it.

I can't become that owner, unfortunately.

@cowtowncoder
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem: it's a big undertaking and so I did not mean to imply I'd expect you to do it -- was more thinking of maybe someone working for, say, AWS, were baseline dependency is to 2.6.7.

My comment suggested I would be open to micro-patch if there is strong demand. But I usually try to first suggest upgrade. For example:

https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.7

points to 2.7.9.3 as version with the fix (and for 2.8, 2.8.11 I think).
But if nothing else is practical, I am willing to make one-off releases. In this case there is just this one fix as far as I can see, so I'd prefer not to make more releases.

@hosamaly
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked AWS to consider this in aws/aws-sdk-java#1597.

Please sign in to comment.