Memory Leak in AuthenticationInterceptor ---------------------------------------- Key: EJBTHREE-558 URL: https://jira.jboss.org/browse/EJBTHREE-558 Project: EJB 3.0 Issue Type: Bug Components: Security Reporter: Luigi Putanesca Assignee: Scott Stark Fix For: Branch_4_2 Attachments: SecurityAssociation.java.patch In org.jboss.aspects.security.AuthenticationInterceptor.invoke(), the finally clause sets SecurityAssociation principal and credential to null. Although I think this was intended to clear any login, what it does is add a SubjectContext with principal = null to the SecurityAssociation.subjectContext stack. This entry is never removed and each time this block of code is invoked, it adds another item to the stack. The stack grows and grows. I think this is mainly a problem when restorePrincipal is true. What also compounds the problem is that if users were logged in by application code and never logged out, they remain on the stack in this thread indefinitely. Not only is this a problem with the stack growing, it is also a security vulnerability because it means there are still authenticated users on the stack that can be restored by another client or process using the same thread. My initial thought on how to fix this was to call the SecurityPrincipal.clear() method in place of setting the principal to null. However I'm not sure it is safe to call clear in that particular place possibly resulting in authentication errors if users are logged out prematurely. Additionally, the 'if' clause there means the code is not always executed so in some cases the clear will not occur. I think the clear needs to be called at some point but I don't know the code well enough to know where the appropriate place would be. Here is the snippet of code in question. See the finally clause: /** * Authenticates the caller using the principal and credentials in the * Infocation if thre is a security manager and an invcocation method. */ public Object invoke(org.jboss.aop.joinpoint.Invocation invocation) throws Throwable { try { authenticate(invocation); } catch (GeneralSecurityException gse) { handleGeneralSecurityException(gse); } Object oldDomain = SecurityContext.currentDomain.get(); try { SecurityContext.currentDomain.set(authenticationManager); return invocation.invokeNext(); } finally { SecurityContext.currentDomain.set(oldDomain); // so that the principal doesn't keep being associated with thread if the thread is pooled SecurityAssociation.popSubjectContext(); if (invocation.getMetaData("security", "principal") != null) { SecurityAssociation.setPrincipal(null); SecurityAssociation.setCredential(null); } } }