Block two more gadgets to exploit default typing issue (c3p0, CVE-201…

…8-7489)

Merged from FasterXML/jackson-databind#1931

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)
 

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

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("'", "\"");
+    }
 }

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);
         }
     }
 
-}
+    // // // 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);
+        }
+    }
+}