From e865a7a4464da63ded9f4b1a2328ad85c9ded78b Mon Sep 17 00:00:00 2001 From: Andrew Tolbert Date: Tue, 12 Dec 2017 17:05:35 -0600 Subject: [PATCH 1/3] Fix #1737 (#1857) --- release-notes/VERSION | 1 + .../deser/BeanDeserializerFactory.java | 11 ++- .../interop/IllegalTypesCheckTest.java | 94 ++++++++++++++++++- 3 files changed, 100 insertions(+), 6 deletions(-) diff --git a/release-notes/VERSION b/release-notes/VERSION index eaf12f3f5b..d5288213aa 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -11,6 +11,7 @@ Project: jackson-databind #1628: Don't print to error stream about failure to load JDK 7 types (reported by Villane@github) #1680: Blacklist couple more types for deserialization +#1737: Block more JDK types from polymorphic deserialization 2.7.9.1 (18-Apr-2017) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java index f2244e0c3d..74af716b41 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java @@ -48,7 +48,7 @@ public class BeanDeserializerFactory static { Set s = new HashSet(); // 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"); @@ -60,6 +60,15 @@ public class BeanDeserializerFactory 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"); + DEFAULT_NO_DESER_CLASS_NAMES = Collections.unmodifiableSet(s); } diff --git a/src/test/java/com/fasterxml/jackson/databind/interop/IllegalTypesCheckTest.java b/src/test/java/com/fasterxml/jackson/databind/interop/IllegalTypesCheckTest.java index 1906eadb6a..8721b9b6ab 100644 --- a/src/test/java/com/fasterxml/jackson/databind/interop/IllegalTypesCheckTest.java +++ b/src/test/java/com/fasterxml/jackson/databind/interop/IllegalTypesCheckTest.java @@ -1,5 +1,6 @@ package com.fasterxml.jackson.databind.interop; +import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.databind.*; /** @@ -12,12 +13,29 @@ static class Bean1599 { public int id; public Object obj; } + + static class PolyWrapper { + @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, + include = JsonTypeInfo.As.WRAPPER_ARRAY) + public Object v; + } - public void testIssue1599() throws Exception + /* + /********************************************************** + /* 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" @@ -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); } } } From e4f83bd3c1c3a379ee7e85902b2bd26f8c3bb160 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 12 Dec 2017 15:07:33 -0800 Subject: [PATCH 2/3] slight change to release notes --- release-notes/VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release-notes/VERSION b/release-notes/VERSION index d5288213aa..cdfc1234b4 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -4,7 +4,7 @@ Project: jackson-databind === Releases === ------------------------------------------------------------------------ -2.7.10 (not yet released) +2.7.9.2 (not yet released) #1607: @JsonIdentityReference not used when setup on class only (reported by vboulaye@github) From f031f27a31625d07922bdd090664c69544200a5d Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 12 Dec 2017 17:15:56 -0800 Subject: [PATCH 3/3] Fix 1/3 of #1855 --- release-notes/VERSION | 1 + .../deser/BeanDeserializerFactory.java | 4 ++- .../interop/IllegalTypesCheckTest.java | 25 ++++++++++++------- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/release-notes/VERSION b/release-notes/VERSION index cdfc1234b4..07c9c69e45 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -12,6 +12,7 @@ Project: jackson-databind (reported by Villane@github) #1680: Blacklist couple more types for deserialization #1737: Block more JDK types from polymorphic deserialization +#1855: (partial) Blacklist for more serialization gadgets (dbcp/tomcat) 2.7.9.1 (18-Apr-2017) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java index 74af716b41..217ffd9c62 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java @@ -68,7 +68,9 @@ public class BeanDeserializerFactory 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); } diff --git a/src/test/java/com/fasterxml/jackson/databind/interop/IllegalTypesCheckTest.java b/src/test/java/com/fasterxml/jackson/databind/interop/IllegalTypesCheckTest.java index 8721b9b6ab..0581242217 100644 --- a/src/test/java/com/fasterxml/jackson/databind/interop/IllegalTypesCheckTest.java +++ b/src/test/java/com/fasterxml/jackson/databind/interop/IllegalTypesCheckTest.java @@ -58,8 +58,15 @@ public void testXalanTypes1599() throws Exception public void testJDKTypes1737() throws Exception { - _testTypes1737(java.util.logging.FileHandler.class); - _testTypes1737(java.rmi.server.UnicastRemoteObject.class); + _testIllegalType(java.util.logging.FileHandler.class); + _testIllegalType(java.rmi.server.UnicastRemoteObject.class); + } + + // // // Tests for [databind#1855] + public void testJDKTypes1855() throws Exception + { + // apparently included by JDK? + _testIllegalType("com.sun.org.apache.bcel.internal.util.ClassLoader"); } // 17-Aug-2017, tatu: Ideally would test handling of 3rd party types, too, @@ -69,22 +76,22 @@ public void testJDKTypes1737() throws Exception /* public void testSpringTypes1737() throws Exception { - _testTypes1737("org.springframework.aop.support.AbstractBeanFactoryPointcutAdvisor"); - _testTypes1737("org.springframework.beans.factory.config.PropertyPathFactoryBean"); + _testIllegalType("org.springframework.aop.support.AbstractBeanFactoryPointcutAdvisor"); + _testIllegalType("org.springframework.beans.factory.config.PropertyPathFactoryBean"); } public void testC3P0Types1737() throws Exception { - _testTypes1737("com.mchange.v2.c3p0.JndiRefForwardingDataSource"); - _testTypes1737("com.mchange.v2.c3p0.WrapperConnectionPoolDataSource"); + _testIllegalType("com.mchange.v2.c3p0.JndiRefForwardingDataSource"); + _testIllegalType("com.mchange.v2.c3p0.WrapperConnectionPoolDataSource"); } */ - private void _testTypes1737(Class nasty) throws Exception { - _testTypes1737(nasty.getName()); + private void _testIllegalType(Class nasty) throws Exception { + _testIllegalType(nasty.getName()); } - private void _testTypes1737(String clsName) throws Exception + private void _testIllegalType(String clsName) throws Exception { // While usually exploited via default typing let's not require // it here; mechanism still the same