diff --git a/release-notes/VERSION b/release-notes/VERSION index dca5939dc..5b299a85b 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -20,6 +20,7 @@ One more patch release for 1.9. * [databind#1735]: Missing type checks when using polymorphic type ids * [databind#1737]: Block more JDK types from polymorphic deserialization [CVE 2017-15095] * [databind#1855]: Blacklist for more serialization gadgets (dbcp/tomcat, spring) +* [databind#1931]: Two more `c3p0` gadgets to exploit default typing issue 1.9.13 (14-Jul-2013) diff --git a/src/mapper/java/org/codehaus/jackson/map/jsontype/impl/SubTypeValidator.java b/src/mapper/java/org/codehaus/jackson/map/jsontype/impl/SubTypeValidator.java index 865c20e75..8e4698862 100644 --- a/src/mapper/java/org/codehaus/jackson/map/jsontype/impl/SubTypeValidator.java +++ b/src/mapper/java/org/codehaus/jackson/map/jsontype/impl/SubTypeValidator.java @@ -17,7 +17,10 @@ */ public class SubTypeValidator { - protected final static String PREFIX_STRING = "org.springframework."; + protected final static String PREFIX_SPRING = "org.springframework."; + + protected final static String PREFIX_C3P0 = "com.mchange.v2.c3p0."; + /** * Set of well-known "nasty classes", deserialization of which is considered dangerous * and should (and is) prevented by default. @@ -49,6 +52,10 @@ public class SubTypeValidator // [databind#1855]: more 3rd party s.add("org.apache.tomcat.dbcp.dbcp2.BasicDataSource"); s.add("com.sun.org.apache.bcel.internal.util.ClassLoader"); + // [databind#1931]; more 3rd party + s.add("com.mchange.v2.c3p0.ComboPooledDataSource"); + s.add("com.mchange.v2.c3p0.debug.AfterCloseLoggingComboPooledDataSource"); + DEFAULT_NO_DESER_CLASS_NAMES = Collections.unmodifiableSet(s); } @@ -78,7 +85,10 @@ public void validateSubType(JavaType type) throws JsonMappingException // 18-Dec-2017, tatu: As per [databind#1855], need bit more sophisticated handling // for some Spring framework types - if (full.startsWith(PREFIX_STRING)) { + // 05-Jan-2017, tatu: ... also, only applies to classes, not interfaces + if (raw.isInterface()) { + ; + } else if (full.startsWith(PREFIX_SPRING)) { 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? @@ -88,6 +98,16 @@ public void validateSubType(JavaType type) throws JsonMappingException break main_check; } } + } else if (full.startsWith(PREFIX_C3P0)) { + // [databind#1737]; more 3rd party + // s.add("com.mchange.v2.c3p0.JndiRefForwardingDataSource"); + // s.add("com.mchange.v2.c3p0.WrapperConnectionPoolDataSource"); + // [databind#1931]; more 3rd party + // com.mchange.v2.c3p0.ComboPooledDataSource + // com.mchange.v2.c3p0.debug.AfterCloseLoggingComboPooledDataSource + if (full.endsWith("DataSource")) { + break main_check; + } } return; } while (false); diff --git a/src/test/org/codehaus/jackson/map/BaseMapTest.java b/src/test/org/codehaus/jackson/map/BaseMapTest.java index 24a32a14e..1aa14c273 100644 --- a/src/test/org/codehaus/jackson/map/BaseMapTest.java +++ b/src/test/org/codehaus/jackson/map/BaseMapTest.java @@ -208,4 +208,8 @@ protected String asJSONObjectValueString(ObjectMapper m, Object... args) } return m.writeValueAsString(map); } + + protected static String aposToQuotes(String json) { + return json.replace("'", "\""); + } } diff --git a/src/test/org/codehaus/jackson/map/interop/TestIllegalTypes.java b/src/test/org/codehaus/jackson/map/interop/TestIllegalTypes.java index b8fe4235a..42538e3f9 100644 --- a/src/test/org/codehaus/jackson/map/interop/TestIllegalTypes.java +++ b/src/test/org/codehaus/jackson/map/interop/TestIllegalTypes.java @@ -11,7 +11,7 @@ */ public class TestIllegalTypes extends BaseMapTest { - public static final String JSON = ( + public static final String JSON = aposToQuotes( "{'id': 124,\n" + " 'obj':[ 'com.sun.org.apache.xalan.internal.xsltc.trax.TemplatesImpl',\n" + " {\n" @@ -20,7 +20,7 @@ public class TestIllegalTypes extends BaseMapTest + " 'outputProperties' : { }\n" + " }\n" + " ]\n" - + "}").replace('\'','"'); + + "}"); public static class Bean1599 { @@ -35,9 +35,16 @@ public static class Bean1599WithAnnotation public Object obj; } + static class PolyWrapper + { + @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, + include = JsonTypeInfo.As.WRAPPER_ARRAY) + public Object v; + } public void testIssue1599() throws Exception { + final String clsName = "com.sun.org.apache.xalan.internal.xsltc.trax.TemplatesImpl"; ObjectMapper mapper = new ObjectMapper(); mapper.enableDefaultTyping(); try @@ -47,14 +54,13 @@ public void testIssue1599() throws Exception } catch (JsonMappingException e) { - verifyException(e, "Illegal type"); - verifyException(e, "to deserialize"); - verifyException(e, "prevented for security reasons"); + _verifySecurityException(e, clsName); } } public void testIssue1599WithAnnotation() throws Exception { + final String clsName = "com.sun.org.apache.xalan.internal.xsltc.trax.TemplatesImpl"; ObjectMapper mapper = new ObjectMapper(); try { @@ -63,10 +69,64 @@ public void testIssue1599WithAnnotation() throws Exception } catch (JsonMappingException e) { - verifyException(e, "Illegal type"); - verifyException(e, "to deserialize"); - verifyException(e, "prevented for security reasons"); + _verifySecurityException(e, clsName); } } -} \ No newline at end of file + // // // Tests for [databind#1855] + public void testJDKTypes1855() throws Exception + { + // apparently included by JDK? + _testIllegalType("com.sun.org.apache.bcel.internal.util.ClassLoader"); + } + + // [databind#1931] + public void testC3P0Types() throws Exception + { + _testIllegalType("com.mchange.v2.c3p0.ComboPooledDataSource"); // [databind#1931] + _testIllegalType("com.mchange.v2.c3p0.debug.AfterCloseLoggingComboPooledDataSource"); + } + + private void _testIllegalType(Class nasty) throws Exception { + _testIllegalType(nasty.getName()); + } + + private void _testIllegalType(String clsName) throws Exception + { + ObjectMapper mapper = new ObjectMapper(); + // 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); + } + } +}