Commons Logging
  1. Commons Logging
  2. LOGGING-130

Potential missing privileged block for class loader

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.1
    • Fix Version/s: 1.1.2
    • Labels:
      None
    • Environment:

      Windows 7 under Sun JRE 6 Update 13, 64-bit
      Running Commons HttpClient 3,1 w/ Commons Logging 1.1.1

      Description

      When attempting to instantiate a HttpClient, a call to LogFactory.getLog() is made. Going deeper, Commons Logging later attempts to make an unprivileged call to java.lang.ClassLoader.getParent(). Under systems with an installed SecurityManager (like mine), this may be forbidden.

      In particular, this call will require the RuntimePermission getClassLoader. In my particular case, I am attempting to sandbox specific segments of code, and thus cannot grant this permission to the user of HttpClient (and, thus, Commons Logging). However, I feel that Commons Logging should be able to trust itself to make a self-checked call to ClassLoader.getParent().

      The stack trace for my situation (trimmed off to assist you) is as follows:

      Caused by: org.apache.commons.logging.LogConfigurationException: java.lang.SecurityException: Cannot request this permission from a tainted execution path (Caused by java.lang.SecurityException: Cannot request this permission from a tainted execution path)
      at org.apache.commons.logging.impl.LogFactoryImpl.newInstance(LogFactoryImpl.java:637)
      at org.apache.commons.logging.impl.LogFactoryImpl.getInstance(LogFactoryImpl.java:336)
      at org.apache.commons.logging.impl.LogFactoryImpl.getInstance(LogFactoryImpl.java:310)
      at org.apache.commons.logging.LogFactory.getLog(LogFactory.java:685)
      at org.apache.commons.httpclient.HttpClient.<clinit>(HttpClient.java:66)
      ... 11 more
      Caused by: java.lang.SecurityException: Cannot request this permission from a tainted execution path
      at com.mmoui.manager.MinionSecurityManager.verifyUntainted(MinionSecurityManager.java:507)
      at com.mmoui.manager.MinionSecurityManager.checkPermission(MinionSecurityManager.java:263)
      at com.mmoui.manager.MinionSecurityManager.checkPermission(MinionSecurityManager.java:474)
      at java.lang.ClassLoader.getParent(ClassLoader.java:1233)
      at org.apache.commons.logging.impl.LogFactoryImpl.getLowestClassLoader(LogFactoryImpl.java:1327)
      at org.apache.commons.logging.impl.LogFactoryImpl.getBaseClassLoader(LogFactoryImpl.java:1247)
      at org.apache.commons.logging.impl.LogFactoryImpl.createLogFromClass(LogFactoryImpl.java:1048)
      at org.apache.commons.logging.impl.LogFactoryImpl.discoverLogImplementation(LogFactoryImpl.java:914)
      at org.apache.commons.logging.impl.LogFactoryImpl.newInstance(LogFactoryImpl.java:604)
      ... 15 more

      I feel that LogFactoryImpl.getLowestClassLoader's call to java.lang.ClassLoader.getParent() should be wrapped by AccessController.doPrivileged(). I can't think of any reason not to do this currently. This would allow my application to trust Commons Logging and not have to grant the RuntimePermission getClassLoader to the users of HttpClient.

      1. LOGGING-130.patch
        1 kB
        Dennis Lundberg

        Issue Links

          Activity

          Matthew P. Del Buono created issue -
          Hide
          Matthew P. Del Buono added a comment - - edited

          I should clarify, the reason for the "tainted execution path" is due to the fact that the caller of HttpClient.<clinit> (hidden by "... 11 more") is untrusted (non-apache) code which I am trying to sandbox. None of the code in org.apache.** nor com.mmoui.manager.** nor java.** can contribute to this concept of "taint" as it is all trusted.

          Show
          Matthew P. Del Buono added a comment - - edited I should clarify, the reason for the "tainted execution path" is due to the fact that the caller of HttpClient.<clinit> (hidden by "... 11 more") is untrusted (non-apache) code which I am trying to sandbox. None of the code in org.apache.** nor com.mmoui.manager.** nor java.** can contribute to this concept of "taint" as it is all trusted.
          Hide
          Dennis Lundberg added a comment -

          Proposed patch that makes use of getParentClassLoader(ClassLoader) that already uses an AccessController.

          Show
          Dennis Lundberg added a comment - Proposed patch that makes use of getParentClassLoader(ClassLoader) that already uses an AccessController.
          Dennis Lundberg made changes -
          Field Original Value New Value
          Attachment LOGGING-130.patch [ 12429101 ]
          Dennis Lundberg made changes -
          Link This issue is duplicated by LOGGING-134 [ LOGGING-134 ]
          sebb committed 1362978 (2 files)
          Hide
          Sebb added a comment -

          Applied patch; same code was already used elsewhere to handle the same situation.

          URL: http://svn.apache.org/viewvc?rev=1362978&view=rev
          Log:
          LOGGING-130 - Potential missing privileged block for class loader

          Modified:
          commons/proper/logging/trunk/RELEASE-NOTES.txt
          commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java

          Show
          Sebb added a comment - Applied patch; same code was already used elsewhere to handle the same situation. URL: http://svn.apache.org/viewvc?rev=1362978&view=rev Log: LOGGING-130 - Potential missing privileged block for class loader Modified: commons/proper/logging/trunk/RELEASE-NOTES.txt commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
          Sebb made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 1.1.2 [ 12314498 ]
          Resolution Fixed [ 1 ]
          Thomas Neidhart made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Unassigned
              Reporter:
              Matthew P. Del Buono
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development