russell@unturf.com
8b04e01458
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.
81 lines
4.3 KiB
Diff
81 lines
4.3 KiB
Diff
# 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);
|
|
+ }
|
|
}
|
|
}
|