From 8b04e014581756e4146e8a7f13443400349dc0db Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Sun, 26 Apr 2026 08:29:50 -0400 Subject: [PATCH] wildfly-0001 UNDF-1305: ElytronSecurityIntegration ThreadLocal leak (MOAD-0003) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 = 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. --- UNDF-REGISTRY.json | 1 + ...ron-securitycontext-threadlocal-leak.patch | 81 ++++++++++++++ ...lytron-securitycontext-threadlocal-leak.md | 105 ++++++++++++++++++ whitepaper/outreach/wildfly.md | 91 +++++++++++++++ 4 files changed, 278 insertions(+) create mode 100644 defects/wildfly/patch/wildfly-0001-elytron-securitycontext-threadlocal-leak.patch create mode 100644 docs/tickets/wildfly-0001-elytron-securitycontext-threadlocal-leak.md create mode 100644 whitepaper/outreach/wildfly.md diff --git a/UNDF-REGISTRY.json b/UNDF-REGISTRY.json index 1707caec6..516f3ccdf 100644 --- a/UNDF-REGISTRY.json +++ b/UNDF-REGISTRY.json @@ -1267,6 +1267,7 @@ "widelands-0001-0001": "UNDF-2026-000000976", "widelands-0002-0002": "UNDF-2026-000000977", "widelands-0003-0003": "UNDF-2026-000000978", + "wildfly-0001": "UNDF-2026-000001305", "wine-0001-0001": "UNDF-2026-000000886", "wine-0002-0002": "UNDF-2026-000001193", "wine-0003-0003": "UNDF-2026-000001194", diff --git a/defects/wildfly/patch/wildfly-0001-elytron-securitycontext-threadlocal-leak.patch b/defects/wildfly/patch/wildfly-0001-elytron-securitycontext-threadlocal-leak.patch new file mode 100644 index 000000000..774c953ae --- /dev/null +++ b/defects/wildfly/patch/wildfly-0001-elytron-securitycontext-threadlocal-leak.patch @@ -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 = 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); ++ } + } + } diff --git a/docs/tickets/wildfly-0001-elytron-securitycontext-threadlocal-leak.md b/docs/tickets/wildfly-0001-elytron-securitycontext-threadlocal-leak.md new file mode 100644 index 000000000..a8a0282a7 --- /dev/null +++ b/docs/tickets/wildfly-0001-elytron-securitycontext-threadlocal-leak.md @@ -0,0 +1,105 @@ +# wildfly-0001: ElytronSecurityIntegration ThreadLocal never cleared + +**Target:** wildfly/wildfly +**Severity:** HIGH +**CWE:** CWE-668 (Exposure of Resource to Wrong Sphere) +**MOAD:** MOAD-0003 (A Leaked Context) +**File:** `connector/src/main/java/org/jboss/as/connector/security/ElytronSecurityIntegration.java:38, 51-53` +**Language:** Java +**Status:** open + +## Description + +`ElytronSecurityIntegration` is the WildFly bridge between the JCA (Java Connector Architecture) `SecurityIntegration` SPI and the Elytron security subsystem. It stores the per-Work-item `SecurityContext` in a `ThreadLocal`: + +```java +private final ThreadLocal securityContext = new ThreadLocal<>(); + +@Override +public SecurityContext getSecurityContext() { + return this.securityContext.get(); +} + +@Override +public void setSecurityContext(SecurityContext context) { + this.securityContext.set(context); +} +``` + +There is **no corresponding `securityContext.remove()` call anywhere in the WildFly codebase** (verified by `grep -rn "securityContext.remove\|securityContext\.set(null\|setSecurityContext(null"`). The ThreadLocal is set per Work item but never cleared. + +JCA's `WorkManager` runs Work items in a thread pool. After Work A on thread T completes, the thread returns to the pool with Work A's `SecurityContext` still bound to it. When Work B picks up thread T, any code path that reads `getSecurityContext()` before B installs its own context sees Work A's identity. + +`WildflyWorkWrapper.runWork()` does exactly that pre-install read: + +```java +@Override +protected void runWork() throws WorkCompletedException { + if (securityIntegration.getSecurityContext() != null) + ((ElytronSecurityContext) securityIntegration.getSecurityContext()).runWork(() -> { ... }); + else super.runWork(); +} +``` + +If a Work item arrives where the caller forgot to set the context (or set it to `null` deliberately), the worker may still execute under Work A's leftover identity instead of falling through to the no-context `super.runWork()` branch. + +## Severity Note + +This is the textbook MOAD-0003 (A Leaked Context) pattern from Elytron. Multi-tenant JCA deployments where Work items run under different principals (per-tenant database connection pools, per-app Resource Adapters, JMS message-driven beans behind Work invocation) are exposed: + +- **Identity leak** — Work B inherits Work A's principal silently +- **Defense-in-depth failure** — even if the caller "always sets context first," any exception path or error before the `setSecurityContext()` call reads the leftover + +ThreadLocal-via-thread-pool is exactly the pattern MOAD-0003 was named for. WildFly's own coding standards advocate `try/finally` discipline around request-scoped state. + +## Root Cause + +```java +// ElytronSecurityIntegration.java:38, 51-53 +private final ThreadLocal securityContext = new ThreadLocal<>(); +// ... +@Override +public void setSecurityContext(SecurityContext context) { + this.securityContext.set(context); // never .remove()'d anywhere +} +``` + +## Fix + +Two-file surgical patch (no SPI change): + +1. `ElytronSecurityIntegration.setSecurityContext(null)` now calls `.remove()`. Callers that pass `null` (already legal per the Nullable convention) get the correct ThreadLocal-clear semantics. + +2. `WildflyWorkWrapper.runWork()` wraps its body in `try { ... } finally { securityIntegration.setSecurityContext(null); }`. The thread is cleared after every Work item, before returning to the pool. + +```java +// ElytronSecurityIntegration.java:51 (5-line change) +public void setSecurityContext(SecurityContext context) { + if (context == null) { + this.securityContext.remove(); + } else { + this.securityContext.set(context); + } +} + +// WildflyWorkWrapper.java:43 (try/finally wrap) +protected void runWork() throws WorkCompletedException { + try { + if (securityIntegration.getSecurityContext() != null) + ((ElytronSecurityContext) securityIntegration.getSecurityContext()).runWork(...); + else super.runWork(); + } finally { + securityIntegration.setSecurityContext(null); + } +} +``` + +## Why this surfaced now + +This finding was buried under 4,840 unmoad MOAD-0003 false positives in earlier wave-26 scanning of WildFly. The unmoad commit `1f48798` (Java ThreadLocal-scoped `.set()` leak detection) restricted the M3 detector to fire only on `.set()` calls whose receiver was previously declared as `ThreadLocal` / `InheritableThreadLocal` / `ScopedValue` / `FastThreadLocal`. WildFly's M3 finding count dropped 4840 → 37, surfacing this and a handful of other real ThreadLocal patterns for triage. + +## Discovery context + +- `wildfly-0001` discovered manually via inspection after unmoad detector enhancement (commit `1f48798`) cleared 99.2% of M3 noise in WildFly +- Scanner did NOT generate this UNDF; it cleared enough noise that human review of the residual 37 findings caught the real defect +- This is the inverse pipeline from typical CWE-407 flagships (where the scanner finds; we triage; we patch). Here the scanner improved its own signal-to-noise so the human triage could find what the scanner alone could not have ranked. diff --git a/whitepaper/outreach/wildfly.md b/whitepaper/outreach/wildfly.md new file mode 100644 index 000000000..66f5e3597 --- /dev/null +++ b/whitepaper/outreach/wildfly.md @@ -0,0 +1,91 @@ +# WildFly — MOAD-0003 (Leaked Context) Disclosure Brief + +**Project:** WildFly (wildfly/wildfly) +**Severity:** HIGH +**CWE:** CWE-668 (Exposure of Resource to Wrong Sphere) +**MOAD:** [MOAD-2026-0003 A Leaked Context](https://undefect.com/moad-2026-0003/) +**Pattern:** ThreadLocal SecurityContext never cleared on JCA Work completion + +## Defect Map + +![]({static}/uploads/intel-wildfly.svg) + +## What it is + +`ElytronSecurityIntegration` is the WildFly bridge between the Java Connector Architecture (JCA) `SecurityIntegration` SPI and the Elytron security subsystem. It stores the per-Work-item `SecurityContext` in a `ThreadLocal` that is **set per Work but never cleared**. + +JCA's `WorkManager` runs Work items in a thread pool. After Work A on thread T completes, the thread returns to the pool with Work A's `SecurityContext` still bound. When Work B picks up thread T, any code path that reads `getSecurityContext()` before B installs its own context sees Work A's identity. + +`WildflyWorkWrapper.runWork()` does exactly the pre-install read: + +```java +if (securityIntegration.getSecurityContext() != null) + ((ElytronSecurityContext) securityIntegration.getSecurityContext()).runWork(...) +``` + +If a Work item arrives where the caller forgot to set the context (or set it to `null` deliberately), the worker may still execute under Work A's leftover identity instead of falling through to the no-context `super.runWork()` branch. + +| Defect | UNDF | +|--------|------| +| `wildfly-0001` | [undf-2026-000001305](../undf-2026-000001305/) | + +## Where it lives + +`connector/src/main/java/org/jboss/as/connector/security/ElytronSecurityIntegration.java:38, 51-53`: + +```java +private final ThreadLocal securityContext = new ThreadLocal<>(); + +@Override +public void setSecurityContext(SecurityContext context) { + this.securityContext.set(context); // never .remove()'d anywhere +} +``` + +Verified by `grep -rn "securityContext.remove\|securityContext\.set(null\|setSecurityContext(null"` — zero matches across the WildFly codebase. + +## Fix + +Two-file surgical patch (no SPI change): + +1. `ElytronSecurityIntegration.setSecurityContext(null)` now calls `.remove()`. Callers passing `null` (already legal per Nullable convention) get the correct ThreadLocal-clear semantics. + +2. `WildflyWorkWrapper.runWork()` wraps its body in `try { ... } finally { securityIntegration.setSecurityContext(null); }`. The thread is cleared after every Work item before returning to the pool. + +```java +// ElytronSecurityIntegration.java:51 (5-line change) +public void setSecurityContext(SecurityContext context) { + if (context == null) { + this.securityContext.remove(); + } else { + this.securityContext.set(context); + } +} + +// WildflyWorkWrapper.java:43 (try/finally wrap) +protected void runWork() throws WorkCompletedException { + try { + if (securityIntegration.getSecurityContext() != null) + ((ElytronSecurityContext) securityIntegration.getSecurityContext()).runWork(...); + else super.runWork(); + } finally { + securityIntegration.setSecurityContext(null); + } +} +``` + +## Why it matters + +WildFly is the JBoss application server family — every Java EE / Jakarta EE deployment using JCA Resource Adapters, message-driven beans, or per-tenant connection pools is exposed: + +- **Identity leak**: Work B inherits Work A's principal if the caller misses the set-before-read window +- **Defense-in-depth failure**: even if the caller "always sets context first," any exception path before the `setSecurityContext()` call reads the leftover +- **Multi-tenant SaaS** running JCA-backed integrations cross-contaminates tenant identities silently + +ThreadLocal-via-thread-pool is exactly the pattern MOAD-0003 was named for. WildFly's own coding standards advocate `try/finally` discipline around request-scoped state. + +## How it surfaced + +This finding was buried under 4,840 unmoad MOAD-0003 false positives in our wave-26 WildFly scan (Wave 26 survey: "wildfly Java JUnit ExtensionContext + Set/EnumSet declared-type FPs"). The unmoad commit `1f48798` (Java ThreadLocal-scoped `.set()` leak detection) restricted the M3 detector to fire only on `.set()` calls whose receiver was previously declared as `ThreadLocal` / `InheritableThreadLocal` / `ScopedValue` / `FastThreadLocal`. WildFly's M3 finding count dropped 4840 → 37, surfacing this real defect for triage. + +This is the inverse pipeline from typical CWE-407 flagships (where the scanner finds; we triage; we patch). Here the scanner improved its own signal-to-noise so the human triage could find what the scanner alone could not have ranked.