russell@unturf.com
2eea7d128c
Acting on the 4 borderline candidates flagged in the session-summary intel. All 4 surfaced after the unmoad scanner enhancements cleared M3/M4 noise. UNDF-1306 wildfly-0002 (HIGH) - ElytronSecurityDomainContextImpl.isValid() sets currentIdentity ThreadLocal with no paired cleanup contract. Subject populated at line 69 is the canonical handover; the ThreadLocal stash leaks to next request on the pool thread. Fix: drop the .set(identity) line. UNDF-1307 wildfly-0003 (LOW) - TransactionRollbackSetupAction.depth.set(null) should be depth.remove() to fully delete the ThreadLocal entry; current pattern leaves null binding pinning the WildFly classloader during undeploy/redeploy. Functional clear, classloader-retention only. UNDF-1308 log4j2-0001 (HIGH) - Log4jMDCAdapter.clear() only clears the log4j ThreadContext map, NOT the SLF4J pushByKey/popByKey stacks (mapOfStacks ThreadLocal). SLF4J spec mandates clear() means "clear all MDC". Per-key Deques accumulate across requests. Fix: add clear() to ThreadLocalMapOfStacks (calls tlMapOfStacks.remove()) and call from the public clear(). UNDF-1309 nakama-0001 (HIGH MOAD-0004) - social/social.go logs OAuth access tokens, ID tokens, oauth2.Token objects (incl. refresh tokens), Steam publisherKey + ticket at debug level via zap.String/zap.Any. 11 call sites. Fix: replace value-logging with shape-logging (token_len, has_token bool) — preserves debug value, redacts secret bytes. First MOAD-0004 patch this session. Companion to the 3 MOAD-0003 patches (wildfly-0001/0002/0003) extending the inverse-pipeline pattern across projects: scanner enhancement -> noise reduction -> human triage finds defects that were buried. Total session flagships: 9 (was 6) — 5 CWE-407 + 3 MOAD-0003 + 1 MOAD-0004.
53 lines
2.6 KiB
Diff
53 lines
2.6 KiB
Diff
# UNDF: UNDF-2026-000001308
|
|
# CWE-668 / MOAD-0003: A Leaked Context — Log4jMDCAdapter.clear() does not clear
|
|
# the per-key stacks ThreadLocal
|
|
#
|
|
# Defect: log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMDCAdapter.java
|
|
# has TWO per-thread state holders:
|
|
# 1. ThreadContext map (log4j-core, the canonical MDC)
|
|
# 2. mapOfStacks: ThreadLocalMapOfStacks (SLF4J's pushByKey/popByKey
|
|
# stack semantics layered on top — line 37)
|
|
#
|
|
# Log4jMDCAdapter.clear() at line 55-57 only calls ThreadContext.clearMap().
|
|
# The adapter's own mapOfStacks ThreadLocal<Map<String, Deque<String>>> is
|
|
# NOT cleared. SLF4J's MDC.clear() spec mandates "clear all MDC state for
|
|
# this thread", but the adapter leaks the per-key stacks across MDC.clear().
|
|
#
|
|
# Web frameworks (Spring, Quarkus, etc.) call MDC.clear() after each request.
|
|
# The per-key Deques accumulate across requests; a peekByKey() / popByKey() /
|
|
# getCopyOfDequeByKey() call after MDC.clear() returns data pushed by a
|
|
# previous request on the same pool thread.
|
|
#
|
|
# Fix: add ThreadLocalMapOfStacks.clear() that does tlMapOfStacks.remove(),
|
|
# and call it from Log4jMDCAdapter.clear() so the SLF4J spec contract holds.
|
|
#
|
|
# This is a security-relevant Leaked Context defect: when caller code uses
|
|
# pushByKey/popByKey to track per-request state (tenant IDs, trace contexts,
|
|
# user roles), the leftover stacks contain prior request's identifiers.
|
|
--- a/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMDCAdapter.java
|
|
+++ b/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMDCAdapter.java
|
|
@@ -54,6 +54,9 @@ public class Log4jMDCAdapter implements MDCAdapter {
|
|
@Override
|
|
public void clear() {
|
|
ThreadContext.clearMap();
|
|
+ // Also clear the SLF4J pushByKey/popByKey stacks. Without this,
|
|
+ // per-key Deques pushed in prior requests remain visible to the
|
|
+ // next request that peekByKey/popByKey/getCopyOfDequeByKey on the
|
|
+ // same pool thread — a per-thread Leaked Context (MOAD-0003).
|
|
+ mapOfStacks.clear();
|
|
}
|
|
|
|
@Override
|
|
@@ -148,6 +151,12 @@ public class Log4jMDCAdapter implements MDCAdapter {
|
|
final Deque<String> deque = tlMapOfStacks.get().get(key);
|
|
return deque != null ? deque.peek() : null;
|
|
}
|
|
+
|
|
+ public void clear() {
|
|
+ // Use remove() instead of set(new HashMap<>()) so the ThreadLocal
|
|
+ // entry itself is deleted, preventing classloader retention in
|
|
+ // app-server thread pools across application redeploys.
|
|
+ tlMapOfStacks.remove();
|
|
+ }
|
|
}
|
|
}
|