javaserverfaces
  1. javaserverfaces
  2. JAVASERVERFACES-2436

Security bug with FacesContext in application startup

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.7
    • Fix Version/s: 2.1.11, 2.2.0-m05
    • Component/s: None
    • Labels:
      None

      Description

      The FacesContext that is made available during application startup is held in a ThreadLocal. The reference is not properly cleaned up.

      So, if a JSF WAR calls FacesContext.getCurrentInstance() during app startup, another WAR can get access to the leftover context and thus get access to the other WAR's resources.

      Here is how we have reproduced this bug:
      1) Set the app server so that there is only one thread used for application initialization. In JBoss AS7, this can be done by starting the server with -Dorg.jboss.server.bootstrap.maxThreads=1. This is just done to make the bug easier to reproduce.
      2) Call FacesContext.getCurrentInstance() in a ServletContextListener for WAR #1.
      3) Call FacesContext.getCurrentInstance() in a ServletContextListener for WAR #2 and see the context from WAR #1.

      I'm attaching a maven project that demonstrates this issue. I've reproduced it on JBoss AS7 with Mojarra 2.1.7.
      To reproduce with the attached project:
      1) mvn install
      2) Copy target/JSFCtxLeak-1.0-SNAPSHOT.war to target/JSFCtxLeak.war
      3) Open JSFCtxLeak.war and edit web.xml
      4) Change <display-name> from "Hello World Example 1" to "Hello World Example 2".
      5) Save web.xml back into JSFCtxLeak.war
      6) Start JBoss AS7 with "standalone -Dorg.jboss.server.bootstrap.maxThreads=1"
      7) Deploy JSFCtxLeak-1.0-SNAPSHOT.war
      8) Deploy JSFCtxLeak-1.0.war

      You will get this for output in the JBoss console:
      11:10:03,137 INFO [org.jboss.as.server.deployment] (MSC service thread 1-1) JBAS015876: Starting deployment of "JSFCtxLeak-1.0-SNAPSHOT.war"
      11:10:03,192 INFO [org.jboss.as.osgi] (MSC service thread 1-1) JBAS011907: Register module: Module "deployment.JSFCtxLeak-1.0-SNAPSHOT.war:main" from Service Module Load
      er
      11:10:03,206 INFO [stdout] (MSC service thread 1-1) FacesContext == null
      11:10:03,213 INFO [javax.enterprise.resource.webcontainer.jsf.config] (MSC service thread 1-1) Initializing Mojarra 2.1.7-jbossorg-1 (20120227-1401) for context '/JSFCtx
      Leak-1.0-SNAPSHOT'
      11:10:03,251 INFO [org.jboss.web] (MSC service thread 1-1) JBAS018210: Registering web context: /JSFCtxLeak-1.0-SNAPSHOT
      11:10:03,270 INFO [org.jboss.as.server] (management-handler-thread - 1) JBAS018559: Deployed "JSFCtxLeak-1.0-SNAPSHOT.war"
      11:10:12,688 INFO [org.jboss.as.repository] (management-handler-thread - 3) JBAS014900: Content added at location C:\as7trunk\jboss-as\build\target\jboss-as-7.2.0.Alpha1
      -SNAPSHOT\standalone\data\content\61\a7969abab127ca218afce89192c492de6f5d92\content
      11:10:12,700 INFO [org.jboss.as.server.deployment] (MSC service thread 1-1) JBAS015876: Starting deployment of "JSFCtxLeak.war"
      11:10:12,763 INFO [org.jboss.as.osgi] (MSC service thread 1-1) JBAS011907: Register module: Module "deployment.JSFCtxLeak.war:main" from Service Module Loader
      11:10:12,772 INFO [stdout] (MSC service thread 1-1) Context Name: Hello World Example 1
      11:10:12,777 INFO [javax.enterprise.resource.webcontainer.jsf.config] (MSC service thread 1-1) Initializing Mojarra 2.1.7-jbossorg-1 (20120227-1401) for context '/JSFCtx
      Leak'
      11:10:12,814 INFO [org.jboss.web] (MSC service thread 1-1) JBAS018210: Registering web context: /JSFCtxLeak
      11:10:12,834 INFO [org.jboss.as.server] (management-handler-thread - 3) JBAS018559: Deployed "JSFCtxLeak.war"

      Notice that at 11:10:12,772 the JSFCtxLeak.war retrieved the context name for JSFCtxLeak-1.0-SNAPSHOT.war, which is a security violation.

      There are a few issues here:
      1) FacesContext.getCurrentInstance() should always return null if it is called before the PostConstructApplicationEvent. In this case, our ServletContextListener is called before Mojarra's ConfigureListener. It does return null in the first app, but does not return null in the second app.

      2) The JSF spec should explicitly state that FacesContext.getCurrentInstance() should not be called from a ServletContextListener. The reason is that the ordering of ServletContextListeners is undefined by the Servlet and JSP specs. In this case, it is undefined because ConfigureListener is declared in a TLD and there is no ordering guarantee between those declared in a TLD and those declared in web.xml.

      3) Mojarra should clean up the FacesContext in a finally block when all listeners of the PostConstructApplicationEvent have been notified. Apparently, this was not done because the second deployment was able to see the FacesContext from the first deployment.

        Activity

        Hide
        rogerk added a comment -

        Changes For trunk (should be similar to branch).

        Show
        rogerk added a comment - Changes For trunk (should be similar to branch).
        Hide
        rogerk added a comment -

        fix version

        Show
        rogerk added a comment - fix version
        Hide
        rogerk added a comment -

        Committed to trunk: Committed revision 10263.
        Committed to MOJARRA_@_1X_ROLLING branch: revision: 10254

        Show
        rogerk added a comment - Committed to trunk: Committed revision 10263. Committed to MOJARRA_@_1X_ROLLING branch: revision: 10254
        Hide
        stuartdouglas added a comment -

        This fix appears to have introduced a class loader leak, as the wrong object is used in the call to remove() on one of the static maps:

        It is a one line fix:

        https://github.com/stuartwdouglas/mojarra/commit/d929222303f46565b60f47ef3a1a4a1ecee2e484#L0R287

        Show
        stuartdouglas added a comment - This fix appears to have introduced a class loader leak, as the wrong object is used in the call to remove() on one of the static maps: It is a one line fix: https://github.com/stuartwdouglas/mojarra/commit/d929222303f46565b60f47ef3a1a4a1ecee2e484#L0R287
        Hide
        rogerk added a comment -

        Thanks for the patch. Can you please test with the jars4jboss.zip file attached to issue:
        http://java.net/jira/browse/JAVASERVERFACES-2477?

        Show
        rogerk added a comment - Thanks for the patch. Can you please test with the jars4jboss.zip file attached to issue: http://java.net/jira/browse/JAVASERVERFACES-2477?

          People

          • Assignee:
            rogerk
            Reporter:
            ssilvert
          • Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: