CVE-2014-8122: Concurrent Execution using Shared Resource with Improper Synchronization ('Race Condition') (CWE-362)
value: Race condition in JBoss Weld before 2.2.8 and 3.x before 3.0.0 Alpha3 allows remote attackers to obtain information from a previous conversation via vectors related to a stale thread state.
CWE description: The product contains a code sequence that can run concurrently with other code, and the code sequence requires temporary, exclusive access to a shared resource, but a timing window exists in which the shared resource can be modified by another code sequence that is operating concurrently.
Published: 2015-02-13T15:59Z
Repository: https://github.com/weld/core
Language: Java
Files changed: impl/src/main/java/org/jboss/weld/logging/ServletLogger.java, impl/src/main/java/org/jboss/weld/servlet/ConversationContextActivator.java, impl/src/main/java/org/jboss/weld/servlet/HttpContextLifecycle.java
Commit date: 2014-12-04 11:24:22 +0100
Commit message: WELD-1802 An exception during context deactivation/dissociation should
not abort further procesing

Diff: diff --git a/impl/src/main/java/org/jboss/weld/logging/ServletLogger.java b/impl/src/main/java/org/jboss/weld/logging/ServletLogger.java
index ad8899ecf..45ec40676 100644
--- a/impl/src/main/java/org/jboss/weld/logging/ServletLogger.java
+++ b/impl/src/main/java/org/jboss/weld/logging/ServletLogger.java
@@ -16,13 +16,10 @@
  */
 package org.jboss.weld.logging;
 
 import static org.jboss.weld.logging.WeldLogger.WELD_PROJECT_CODE;
 
-import javax.enterprise.context.spi.Context;
-import javax.servlet.http.HttpServletRequest;
-
 import org.jboss.logging.Logger;
 import org.jboss.logging.Logger.Level;
 import org.jboss.logging.annotations.Cause;
 import org.jboss.logging.annotations.LogMessage;
 import org.jboss.logging.annotations.Message;
@@ -58,12 +55,12 @@ public interface ServletLogger extends WeldLogger {
     @LogMessage(level = Level.WARN)
     @Message(id = 711, value = "Context activation pattern {0} ignored as it is overriden by the integrator.", format = Format.MESSAGE_FORMAT)
     void webXmlMappingPatternIgnored(String pattern);
 
     @LogMessage(level = Level.WARN)
-    @Message(id = 712, value = "Unable to dissociate context {0} when destroying request {1}", format = Format.MESSAGE_FORMAT)
-    void unableToDissociateContext(Context context, HttpServletRequest request);
+    @Message(id = 712, value = "Unable to dissociate context {0} from the storage {1}", format = Format.MESSAGE_FORMAT)
+    void unableToDissociateContext(Object context, Object storage);
 
     @Message(id = 713, value = "Unable to inject ServletContext. None is associated with {0}, {1}", format = Format.MESSAGE_FORMAT)
     IllegalStateException cannotInjectServletContext(ClassLoader classLoader, ServletContextService service);
 
     @LogMessage(level = Level.WARN)
@@ -76,6 +73,10 @@ public interface ServletLogger extends WeldLogger {
 
     @LogMessage(level = Level.INFO)
     @Message(id = 716, value = "Running in Servlet 2.x environment. Asynchronous request support is disabled.")
     void servlet2Environment();
 
+    @LogMessage(level = Level.WARN)
+    @Message(id = 717, value = "Unable to deactivate context {0} when destroying request {1}", format = Format.MESSAGE_FORMAT)
+    void unableToDeactivateContext(Object context, Object request);
+
 }
\ No newline at end of file
diff --git a/impl/src/main/java/org/jboss/weld/servlet/ConversationContextActivator.java b/impl/src/main/java/org/jboss/weld/servlet/ConversationContextActivator.java
index ba0967c04..6c9b3846a 100644
--- a/impl/src/main/java/org/jboss/weld/servlet/ConversationContextActivator.java
+++ b/impl/src/main/java/org/jboss/weld/servlet/ConversationContextActivator.java
@@ -136,34 +136,39 @@ public class ConversationContextActivator {
         }
         return (Boolean) result;
     }
 
     protected void deactivateConversationContext(HttpServletRequest request) {
-        ConversationContext conversationContext = httpConversationContext();
-        if (conversationContext.isActive()) {
-            // Only deactivate the context if one is already active, otherwise we get Exceptions
-            if (conversationContext instanceof LazyHttpConversationContextImpl) {
-                LazyHttpConversationContextImpl lazyConversationContext = (LazyHttpConversationContextImpl) conversationContext;
-                if (!lazyConversationContext.isInitialized()) {
-                    // if this lazy conversation has not been touched yet, just deactivate it
-                    lazyConversationContext.deactivate();
-                    return;
+        try {
+            ConversationContext conversationContext = httpConversationContext();
+            if (conversationContext.isActive()) {
+                // Only deactivate the context if one is already active, otherwise we get Exceptions
+                if (conversationContext instanceof LazyHttpConversationContextImpl) {
+                    LazyHttpConversationContextImpl lazyConversationContext = (LazyHttpConversationContextImpl) conversationContext;
+                    if (!lazyConversationContext.isInitialized()) {
+                        // if this lazy conversation has not been touched yet, just deactivate it
+                        lazyConversationContext.deactivate();
+                        return;
+                    }
                 }
-            }
-            boolean isTransient = conversationContext.getCurrentConversation().isTransient();
-            if (ConversationLogger.LOG.isTraceEnabled()) {
+                boolean isTransient = conversationContext.getCurrentConversation().isTransient();
+                if (ConversationLogger.LOG.isTraceEnabled()) {
+                    if (isTransient) {
+                        ConversationLogger.LOG.cleaningUpTransientConversation();
+                    } else {
+                        ConversationLogger.LOG.cleaningUpConversation(conversationContext.getCurrentConversation().getId());
+                    }
+                }
+                conversationContext.invalidate();
+                conversationContext.deactivate();
                 if (isTransient) {
-                    ConversationLogger.LOG.cleaningUpTransientConversation();
-                } else {
-                    ConversationLogger.LOG.cleaningUpConversation(conversationContext.getCurrentConversation().getId());
+                    conversationDestroyedEvent.fire(request);
                 }
             }
-            conversationContext.invalidate();
-            conversationContext.deactivate();
-            if (isTransient) {
-                conversationDestroyedEvent.fire(request);
-            }
+        } catch (Exception e) {
+            ServletLogger.LOG.unableToDeactivateContext(httpConversationContext(), request);
+            ServletLogger.LOG.catchingDebug(e);
         }
     }
 
     protected void disassociateConversationContext(HttpServletRequest request) {
         try {
diff --git a/impl/src/main/java/org/jboss/weld/servlet/HttpContextLifecycle.java b/impl/src/main/java/org/jboss/weld/servlet/HttpContextLifecycle.java
index b3d2d09d1..f16bfef56 100644
--- a/impl/src/main/java/org/jboss/weld/servlet/HttpContextLifecycle.java
+++ b/impl/src/main/java/org/jboss/weld/servlet/HttpContextLifecycle.java
@@ -21,10 +21,12 @@ import javax.servlet.ServletRequestListener;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpSession;
 
 import org.jboss.weld.Container;
 import org.jboss.weld.bootstrap.api.Service;
+import org.jboss.weld.context.BoundContext;
+import org.jboss.weld.context.ManagedContext;
 import org.jboss.weld.context.cache.RequestScopedCache;
 import org.jboss.weld.context.http.HttpRequestContext;
 import org.jboss.weld.context.http.HttpRequestContextImpl;
 import org.jboss.weld.context.http.HttpSessionContext;
 import org.jboss.weld.context.http.HttpSessionDestructionContext;
@@ -277,28 +279,25 @@ public class HttpContextLifecycle implements Service {
              * as it will be invalidated at the end of the async operation.
              */
             if (!servletApi.isAsyncSupported() || !servletApi.isAsyncStarted(request)) {
                 getRequestContext().invalidate();
             }
-            getRequestContext().deactivate();
+
+            safelyDeactivate(getRequestContext(),  request);
             // fire @Destroyed(RequestScoped.class)
             requestDestroyedEvent.fire(request);
-            getSessionContext().deactivate();
+
+            safelyDeactivate(getSessionContext(), request);
             // fire @Destroyed(SessionScoped.class)
             if (!getSessionContext().isValid()) {
                 sessionDestroyedEvent.fire((HttpSession) request.getAttribute(HTTP_SESSION));
             }
         } finally {
-            getRequestContext().dissociate(request);
-
+            safelyDissociate(getRequestContext(), request);
             // WFLY-1533 Underlying HTTP session may be invalid
-            try {
-                getSessionContext().dissociate(request);
-            } catch (Exception e) {
-                ServletLogger.LOG.unableToDissociateContext(getSessionContext(), request);
-                ServletLogger.LOG.catchingDebug(e);
-            }
+            safelyDissociate(getSessionContext(), request);
+
             // Catch block is inside the activator method so that we're able to log the context
             conversationContextActivator.disassociateConversationContext(request);
 
             SessionHolder.clear();
         }
@@ -310,10 +309,14 @@ public class HttpContextLifecycle implements Service {
 
     public void setConversationActivationEnabled(boolean conversationActivationEnabled) {
         this.conversationActivationEnabled = conversationActivationEnabled;
     }
 
+    @Override
+    public void cleanup() {
+    }
+
     /**
      * Some Servlet containers fire HttpServletListeners for include requests (inner requests caused by calling the include method of RequestDispatcher). This
      * causes problems with context shut down as context manipulation is not reentrant. This method detects if this request is an included request or not.
      */
     private boolean isIncludedRequest(HttpServletRequest request) {
@@ -336,9 +339,24 @@ public class HttpContextLifecycle implements Service {
      */
     private boolean isRequestDestroyed(HttpServletRequest request) {
         return request.getAttribute(REQUEST_DESTROYED) != null;
     }
 
-    @Override
-    public void cleanup() {
+    private <T> void safelyDissociate(BoundContext<T> context, T storage) {
+        try {
+            context.dissociate(storage);
+        } catch(Exception e) {
+            ServletLogger.LOG.unableToDissociateContext(context, storage);
+            ServletLogger.LOG.catchingDebug(e);
+        }
+    }
+
+    private void safelyDeactivate(ManagedContext context, HttpServletRequest request) {
+        try {
+            context.deactivate();
+        } catch(Exception e) {
+            ServletLogger.LOG.unableToDeactivateContext(context, request);
+            ServletLogger.LOG.catchingDebug(e);
+        }
     }
+
 }

Vulnerable code: import javax.enterprise.context.spi.Context;
import javax.servlet.http.HttpServletRequest;

    @Message(id = 712, value = "Unable to dissociate context {0} when destroying request {1}", format = Format.MESSAGE_FORMAT)
    void unableToDissociateContext(Context context, HttpServletRequest request);
        ConversationContext conversationContext = httpConversationContext();
        if (conversationContext.isActive()) {
            // Only deactivate the context if one is already active, otherwise we get Exceptions
            if (conversationContext instanceof LazyHttpConversationContextImpl) {
                LazyHttpConversationContextImpl lazyConversationContext = (LazyHttpConversationContextImpl) conversationContext;
                if (!lazyConversationContext.isInitialized()) {
                    // if this lazy conversation has not been touched yet, just deactivate it
                    lazyConversationContext.deactivate();
                    return;
            }
            boolean isTransient = conversationContext.getCurrentConversation().isTransient();
            if (ConversationLogger.LOG.isTraceEnabled()) {
                    ConversationLogger.LOG.cleaningUpTransientConversation();
                } else {
                    ConversationLogger.LOG.cleaningUpConversation(conversationContext.getCurrentConversation().getId());
            conversationContext.invalidate();
            conversationContext.deactivate();
            if (isTransient) {
                conversationDestroyedEvent.fire(request);
            }
            getRequestContext().deactivate();
            getSessionContext().deactivate();
            getRequestContext().dissociate(request);

            try {
                getSessionContext().dissociate(request);
            } catch (Exception e) {
                ServletLogger.LOG.unableToDissociateContext(getSessionContext(), request);
                ServletLogger.LOG.catchingDebug(e);
            }
    @Override
    public void cleanup() {
Fixed code:     @Message(id = 712, value = "Unable to dissociate context {0} from the storage {1}", format = Format.MESSAGE_FORMAT)
    void unableToDissociateContext(Object context, Object storage);
    @LogMessage(level = Level.WARN)
    @Message(id = 717, value = "Unable to deactivate context {0} when destroying request {1}", format = Format.MESSAGE_FORMAT)
    void unableToDeactivateContext(Object context, Object request);

        try {
            ConversationContext conversationContext = httpConversationContext();
            if (conversationContext.isActive()) {
                // Only deactivate the context if one is already active, otherwise we get Exceptions
                if (conversationContext instanceof LazyHttpConversationContextImpl) {
                    LazyHttpConversationContextImpl lazyConversationContext = (LazyHttpConversationContextImpl) conversationContext;
                    if (!lazyConversationContext.isInitialized()) {
                        // if this lazy conversation has not been touched yet, just deactivate it
                        lazyConversationContext.deactivate();
                        return;
                    }
                boolean isTransient = conversationContext.getCurrentConversation().isTransient();
                if (ConversationLogger.LOG.isTraceEnabled()) {
                    if (isTransient) {
                        ConversationLogger.LOG.cleaningUpTransientConversation();
                    } else {
                        ConversationLogger.LOG.cleaningUpConversation(conversationContext.getCurrentConversation().getId());
                    }
                }
                conversationContext.invalidate();
                conversationContext.deactivate();
                    conversationDestroyedEvent.fire(request);
        } catch (Exception e) {
            ServletLogger.LOG.unableToDeactivateContext(httpConversationContext(), request);
            ServletLogger.LOG.catchingDebug(e);
import org.jboss.weld.context.BoundContext;
import org.jboss.weld.context.ManagedContext;

            safelyDeactivate(getRequestContext(),  request);

            safelyDeactivate(getSessionContext(), request);
            safelyDissociate(getRequestContext(), request);
            safelyDissociate(getSessionContext(), request);

    @Override
    public void cleanup() {
    }

    private <T> void safelyDissociate(BoundContext<T> context, T storage) {
        try {
            context.dissociate(storage);
        } catch(Exception e) {
            ServletLogger.LOG.unableToDissociateContext(context, storage);
            ServletLogger.LOG.catchingDebug(e);
        }
    }

    private void safelyDeactivate(ManagedContext context, HttpServletRequest request) {
        try {
            context.deactivate();
        } catch(Exception e) {
            ServletLogger.LOG.unableToDeactivateContext(context, request);
            ServletLogger.LOG.catchingDebug(e);
        }