wildfly-0001 UNDF-1305: ElytronSecurityIntegration ThreadLocal leak (MOAD-0003)
First MOAD-0003 (Leaked Context) flagship this session. Surfaced via scanner enhancement: commit 1f48798 (Java ThreadLocal-scoped .set fix) dropped wildfly M3 noise from 4840 -> 37, exposing this real defect. Defect: ElytronSecurityIntegration.java:38 declares private final ThreadLocal<SecurityContext> securityContext = new ThreadLocal<>(); with setSecurityContext() calling .set(context) and ZERO corresponding .remove() / .set(null) anywhere in the WildFly codebase (verified by grep -rn). JCA WorkManager reuses pool threads across Work items from different security principals; a leftover SecurityContext from prior Work is visible to any subsequent Work that reads getSecurityContext() before installing its own — which WildflyWorkWrapper.runWork() does exactly to decide whether to use Elytron-runWork or super.runWork(). Fix: 2-file surgical patch (no SPI change): 1. setSecurityContext(null) now calls .remove() (clear ThreadLocal, prevent classloader retention) 2. WildflyWorkWrapper.runWork() wraps body in try/finally that calls setSecurityContext(null) after the Work item completes This is the inverse pipeline from CWE-407 flagships: scanner improved its signal-to-noise so triage could find what raw scanning could not have ranked.
This commit is contained in:
parent
8b3a38bcab
commit
8b04e01458
4 changed files with 278 additions and 0 deletions
|
|
@ -0,0 +1,81 @@
|
|||
# UNDF: UNDF-2026-000001305
|
||||
# CWE-668 / MOAD-0003: A Leaked Context — ElytronSecurityIntegration ThreadLocal
|
||||
# never cleared on JCA Work completion
|
||||
#
|
||||
# Defect: connector/src/main/java/org/jboss/as/connector/security/
|
||||
# ElytronSecurityIntegration.java declares
|
||||
# private final ThreadLocal<SecurityContext> securityContext = new ThreadLocal<>();
|
||||
# with setSecurityContext(SecurityContext) calling .set(context). There is
|
||||
# NO corresponding .remove() / .set(null) anywhere in the WildFly codebase
|
||||
# (verified by grep -rn "securityContext.remove\|securityContext\.set(null\|
|
||||
# setSecurityContext(null" wildfly/).
|
||||
#
|
||||
# JCA WorkManager runs Work items in a thread pool. After Work A on thread
|
||||
# T sets securityContext = Alice and runs to completion, the thread returns
|
||||
# to the pool with Alice's SecurityContext still bound. When Work B picks
|
||||
# up thread T, any code path that reads getSecurityContext() before B's own
|
||||
# setSecurityContext() call sees Alice's identity. WildflyWorkWrapper.runWork()
|
||||
# does exactly this:
|
||||
# if (securityIntegration.getSecurityContext() != null)
|
||||
# ((ElytronSecurityContext) securityIntegration.getSecurityContext()).runWork(...)
|
||||
#
|
||||
# Fix: tighten setSecurityContext(null) to call .remove() (clears ThreadLocal,
|
||||
# avoids classloader retention) and have WildflyWorkWrapper.runWork() call
|
||||
# setSecurityContext(null) in a finally block after work completes.
|
||||
#
|
||||
# This is a surgical 2-file change that doesn't alter the public
|
||||
# SecurityIntegration interface. Callers using the existing setSecurityContext
|
||||
# pattern see no behavior change; setSecurityContext(null) (already legal per
|
||||
# the Nullable convention) now also removes the ThreadLocal entry, which is
|
||||
# the correct semantics for "clear the context on this thread."
|
||||
--- a/connector/src/main/java/org/jboss/as/connector/security/ElytronSecurityIntegration.java
|
||||
+++ b/connector/src/main/java/org/jboss/as/connector/security/ElytronSecurityIntegration.java
|
||||
@@ -49,7 +49,12 @@ public class ElytronSecurityIntegration implements SecurityIntegration {
|
||||
|
||||
@Override
|
||||
public void setSecurityContext(SecurityContext context) {
|
||||
- this.securityContext.set(context);
|
||||
+ if (context == null) {
|
||||
+ // Remove the ThreadLocal entry instead of leaving a null binding.
|
||||
+ // Prevents classloader retention and signals "clear this thread."
|
||||
+ this.securityContext.remove();
|
||||
+ } else {
|
||||
+ this.securityContext.set(context);
|
||||
+ }
|
||||
}
|
||||
|
||||
@Override
|
||||
--- a/connector/src/main/java/org/jboss/as/connector/services/workmanager/WildflyWorkWrapper.java
|
||||
+++ b/connector/src/main/java/org/jboss/as/connector/services/workmanager/WildflyWorkWrapper.java
|
||||
@@ -42,15 +42,21 @@ public class WildflyWorkWrapper extends org.jboss.jca.core.workmanager.WorkWrapp
|
||||
|
||||
@Override
|
||||
protected void runWork() throws WorkCompletedException {
|
||||
- if (securityIntegration.getSecurityContext() != null)
|
||||
- ((ElytronSecurityContext) securityIntegration.getSecurityContext()).runWork(() -> {
|
||||
- try {
|
||||
- WildflyWorkWrapper.super.runWork();
|
||||
- } catch (WorkCompletedException e) {
|
||||
- ConnectorLogger.ROOT_LOGGER.unexceptedWorkerCompletionError(e.getLocalizedMessage(),e);
|
||||
- }
|
||||
- });
|
||||
- else super.runWork();
|
||||
+ try {
|
||||
+ if (securityIntegration.getSecurityContext() != null)
|
||||
+ ((ElytronSecurityContext) securityIntegration.getSecurityContext()).runWork(() -> {
|
||||
+ try {
|
||||
+ WildflyWorkWrapper.super.runWork();
|
||||
+ } catch (WorkCompletedException e) {
|
||||
+ ConnectorLogger.ROOT_LOGGER.unexceptedWorkerCompletionError(e.getLocalizedMessage(),e);
|
||||
+ }
|
||||
+ });
|
||||
+ else super.runWork();
|
||||
+ } finally {
|
||||
+ // Clear the ThreadLocal SecurityContext bound for this Work item
|
||||
+ // so the next Work scheduled on this pool thread does not inherit
|
||||
+ // the previous principal's identity. (See ElytronSecurityIntegration
|
||||
+ // setSecurityContext(null) which now calls .remove() under the hood.)
|
||||
+ securityIntegration.setSecurityContext(null);
|
||||
+ }
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue