Address security concerns surrounding overliberal XML parsing, CVE-20…

…18-20433, https://nvd.nist.gov/vuln/detail/CVE-2018-20433. Thanks to zhutougg on GitHub.

src/doc/index.html

@@ -2738,7 +2738,7 @@ <h3>
 	or in a <a href="#c3p0_conf">HOCON (typesafe-config) file</a>.
       </p>
       <div class="other_properties_desc">
-    <h4><a name="locating_configuration_information">Locating Configuration Information</a></h4>
+    <h4><a name="locating_configuration_information">Locating and Resolving Configuration Information</a></h4>
     <p>
       Normally, c3p0's configuration information is placed in a either a c3p0-config.xml or c3p0.properties file
       at the top-level of an application's CLASSPATH. However, if you wish to place configuration information
@@ -2752,6 +2752,17 @@ <h4><a name="locating_configuration_information">Locating Configuration Informat
     <p>
       If you set this property to a value beginning with "<tt>classloader:</tt>", c3p0 will search for an XML config file as a ClassLoader resource,
       that is, in any location you specify under your classpath, including jar-file <tt>META-INF</tt> directories.
+    </p>
+    <p>
+      Due to <a href="">security concerns surrounding liberal parsing of XML references</a>, as of c3p0-0.9.5.3, c3p0 by default <i>no longer expands entity references in XML config files</i>.
+      However, installations that understand the full transitive closure of all entity references in their XML config may override this conservative behavior by setting the following property
+      to <tt>true</tt>:
+    </p>
+    <ul class="other_props_list">
+      <li>com.mchange.v2.c3p0.cfg.xml.expandEntityReferences</li>
+    </ul>
+    <p>
+      This will restore the traditional, liberal parsing of XML config files that c3p0 utilized by default in versions prior to c3p0-0.9.5.3.
     </p>
 	<h4>Logging-related properties</h4>
 	<p>

src/java/com/mchange/v2/c3p0/cfg/C3P0ConfigXmlUtils.java

@@ -115,7 +115,13 @@ private final static void warnCommonXmlConfigResourceMisspellings()
 
     }
 
-    public static C3P0Config extractXmlConfigFromDefaultResource() throws Exception
+    // thanks to zhutougg on GitHub https://github.com/zhutougg/c3p0/commit/2eb0ea97f745740b18dd45e4a909112d4685f87b
+    // let's address hazards associated with overliberal parsing of XML, CVE-2018-20433
+    //
+    // by default entity references will not be expanded, but callers can specify expansion if they wish (important
+    // to retain backwards compatibility with existing config files where users understand the risks)
+
+    public static C3P0Config extractXmlConfigFromDefaultResource( boolean expandEntityReferences ) throws Exception
     {
         InputStream is = null;
 
@@ -128,7 +134,7 @@ public static C3P0Config extractXmlConfigFromDefaultResource() throws Exception
                 return null;
             }
             else
-                return extractXmlConfigFromInputStream( is );
+                return extractXmlConfigFromInputStream( is, expandEntityReferences );
         }
         finally
         {
@@ -141,9 +147,12 @@ public static C3P0Config extractXmlConfigFromDefaultResource() throws Exception
         }
     }
 
-    public static C3P0Config extractXmlConfigFromInputStream(InputStream is) throws Exception
+    public static C3P0Config extractXmlConfigFromInputStream(InputStream is, boolean expandEntityReferences) throws Exception
     {
         DocumentBuilderFactory fact = DocumentBuilderFactory.newInstance();
+
+	fact.setExpandEntityReferences( expandEntityReferences );
+
         DocumentBuilder db = fact.newDocumentBuilder();
         Document doc = db.parse( is );
 
@@ -253,6 +262,22 @@ private static HashMap extractPropertiesFromLevel(Element elem)
         return out;
     }
 
+    /*
+    // preserve old public API, but with security-conservative defaults now
+    // i don't think this API is used by anyone, so I'm gonna leave this commented out unless
+    // somebody complains
+
+    public static C3P0Config extractXmlConfigFromDefaultResource() throws Exception
+    {
+	return extractXmlConfigFromDefaultResource( false );
+    }
+
+    public static C3P0Config extractXmlConfigFromInputStream(InputStream is) throws Exception
+    {
+	return extractXmlConfigFromInputStream( is, false )
+    }
+    */
+
     private C3P0ConfigXmlUtils()
     {}
 }

src/java/com/mchange/v2/c3p0/cfg/DefaultC3P0ConfigFinder.java

@@ -43,8 +43,9 @@
 
 public class DefaultC3P0ConfigFinder implements C3P0ConfigFinder
 {
-    final static String XML_CFG_FILE_KEY            = "com.mchange.v2.c3p0.cfg.xml";
-    final static String CLASSLOADER_RESOURCE_PREFIX = "classloader:";
+    final static String XML_CFG_FILE_KEY               = "com.mchange.v2.c3p0.cfg.xml";
+    final static String XML_CFG_EXPAND_ENTITY_REFS_KEY = "com.mchange.v2.c3p0.cfg.xml.expandEntityReferences";
+    final static String CLASSLOADER_RESOURCE_PREFIX    = "classloader:";
 
     final static MLogger logger = MLog.getLogger( DefaultC3P0ConfigFinder.class );
 
@@ -68,9 +69,11 @@ public C3P0Config findConfig() throws Exception
 	flatDefaults.putAll( C3P0ConfigUtils.extractC3P0PropertiesResources() );
 
 	String cfgFile = C3P0Config.getPropsFileConfigProperty( XML_CFG_FILE_KEY );
+	boolean expandEntityReferences = findExpandEntityReferences();
+
 	if (cfgFile == null)
 	    {
-		C3P0Config xmlConfig = C3P0ConfigXmlUtils.extractXmlConfigFromDefaultResource();
+		C3P0Config xmlConfig = C3P0ConfigXmlUtils.extractXmlConfigFromDefaultResource( expandEntityReferences );
 		if (xmlConfig != null)
 		    {
 			insertDefaultsUnderNascentConfig( flatDefaults, xmlConfig );
@@ -111,7 +114,7 @@ public C3P0Config findConfig() throws Exception
 			    mbOverrideWarning( "file", cfgFile );
 			}
 
-			C3P0Config xmlConfig = C3P0ConfigXmlUtils.extractXmlConfigFromInputStream( is );
+			C3P0Config xmlConfig = C3P0ConfigXmlUtils.extractXmlConfigFromInputStream( is, expandEntityReferences );
 			insertDefaultsUnderNascentConfig( flatDefaults, xmlConfig );
 			out = xmlConfig;
 		    }
@@ -142,4 +145,17 @@ private void mbOverrideWarning( String srcType, String srcName )
 	if ( warn_of_xml_overrides && logger.isLoggable( MLevel.WARNING ) )
 	    logger.log( MLevel.WARNING, "Configuation defined in " + srcType + "'" + srcName + "' overrides all other c3p0 config." );
     }
+
+    private boolean findExpandEntityReferences()
+    {
+	String check = C3P0Config.getPropsFileConfigProperty( XML_CFG_EXPAND_ENTITY_REFS_KEY );
+	boolean out = (check != null && check.trim().equalsIgnoreCase("true"));
+	if ( out && logger.isLoggable( MLevel.WARNING ) )
+	    logger.log( MLevel.WARNING,
+			"Configuration property '" + XML_CFG_EXPAND_ENTITY_REFS_KEY + "' is set to 'true'. " +
+			"Entity references will be resolved in XML c3p0 configuration files. This may be a security hazard. " +
+			"Be sure you understand your XML config files, including the full transitive closure of entity references. " +
+			"See CVE-2018-20433, https://nvd.nist.gov/vuln/detail/CVE-2018-20433");
+	return out;
+    }
 }