+ * Note that returning an object fetched via an AccessController would + * technically be a security flaw anyway; untrusted code that has access + * to a trusted JCL library could use it to fetch the classloader for + * a class even when forbidden to do so directly. * * @since 1.1 */ @@ -771,6 +800,33 @@ public abstract class LogFactory { } } + /** + * Returns the current context classloader. + *
+ * In versions prior to 1.1, this method did not use an AccessController. + * In version 1.1, an AccessController wrapper was incorrectly added to + * this method, causing a minor security flaw. + *
+ * In version 1.1.1 this change was reverted; this method no longer uses + * an AccessController. User code wishing to obtain the context classloader + * must invoke this method via AccessController.doPrivileged if it needs + * support for that. + * + * @return the context classloader associated with the current thread, + * or null if security doesn't allow it. + * + * @throws LogConfigurationException if there was some weird error while + * attempting to get the context classloader. + * + * @throws SecurityException if the current java security policy doesn't + * allow this class to access the context classloader. + */ + protected static ClassLoader getContextClassLoader() + throws LogConfigurationException { + + return directGetContextClassLoader(); + } + /** * Calls LogFactory.directGetContextClassLoader under the control of an * AccessController class. This means that java code running under a @@ -789,9 +845,8 @@ public abstract class LogFactory { * @throws SecurityException if the current java security policy doesn't * allow this class to access the context classloader. */ - protected static ClassLoader getContextClassLoader() - throws LogConfigurationException { - + private static ClassLoader getContextClassLoaderInternal() + throws LogConfigurationException { return (ClassLoader)AccessController.doPrivileged( new PrivilegedAction() { public Object run() { @@ -804,8 +859,8 @@ public abstract class LogFactory { * Return the thread context class loader if available; otherwise return * null. *
- * Most/all code should call getContextClassLoader rather than calling - * this method directly. + * Most/all code should call getContextClassLoaderInternal rather than + * calling this method directly. *
* The thread context class loader is available for JDK 1.2 * or later, if certain security conditions are met. @@ -1480,6 +1535,25 @@ public abstract class LogFactory { return props; } + /** + * Read the specified system property, using an AccessController so that + * the property can be read if JCL has been granted the appropriate + * security rights even if the calling code has not. + *
+ * Take care not to expose the value returned by this method to the + * calling application in any way; otherwise the calling app can use that + * info to access data that should not be available to it. + */ + private static String getSystemProperty(final String key, final String def) + throws SecurityException { + return (String) AccessController.doPrivileged( + new PrivilegedAction() { + public Object run() { + return System.getProperty(key, def); + } + }); + } + /** * Determines whether the user wants internal diagnostic output. If so, * returns an appropriate writer object. Users can enable diagnostic @@ -1489,7 +1563,7 @@ public abstract class LogFactory { private static void initDiagnostics() { String dest; try { - dest = System.getProperty(DIAGNOSTICS_DEST_PROPERTY); + dest = getSystemProperty(DIAGNOSTICS_DEST_PROPERTY, null); if (dest == null) { return; } @@ -1612,6 +1686,9 @@ public abstract class LogFactory { } try { + // Deliberately use System.getProperty here instead of getSystemProperty; if + // the overall security policy for the calling application forbids access to + // these variables then we do not want to output them to the diagnostic stream. logDiagnostic("[ENV] Extension directories (java.ext.dir): " + System.getProperty("java.ext.dir")); logDiagnostic("[ENV] Application classpath (java.class.path): " + System.getProperty("java.class.path")); } catch(SecurityException ex) { @@ -1705,19 +1782,6 @@ public abstract class LogFactory { } } - // called from static class initialiser, ie when class is loaded - private static void initClass() { - // note: it's safe to call methods before initDiagnostics (though - // diagnostic output gets discarded). - thisClassLoader = getClassLoader(LogFactory.class); - initDiagnostics(); - logClassLoaderEnvironment(LogFactory.class); - factories = createFactoryStore(); - if (isDiagnosticsEnabled()) { - logDiagnostic("BOOTSTRAP COMPLETED"); - } - } - // ---------------------------------------------------------------------- // Static initialiser block to perform initialisation at class load time. // @@ -1738,12 +1802,14 @@ public abstract class LogFactory { // ---------------------------------------------------------------------- static { - AccessController.doPrivileged( - new PrivilegedAction() { - public Object run() { - initClass(); - return null; - } - }); + // note: it's safe to call methods before initDiagnostics (though + // diagnostic output gets discarded). + thisClassLoader = getClassLoader(LogFactory.class); + initDiagnostics(); + logClassLoaderEnvironment(LogFactory.class); + factories = createFactoryStore(); + if (isDiagnosticsEnabled()) { + logDiagnostic("BOOTSTRAP COMPLETED"); + } } }