-
Notifications
You must be signed in to change notification settings - Fork 6
Description
Bug Description
In org.seasar.dbflute.AccessContext, a static ThreadLocal (_defaultThreadLocal) is used to store sensitive database access context, including AccessUser and AccessModule. However, the framework lacks an internal, fail-safe cleanup mechanism and relies entirely on developers to manually clear it via interceptors. Furthermore, the cleanup method itself does not properly remove the ThreadLocal variable. This design exposes applications to severe Context/Identity Pollution (Type III UTL) in thread-pool environments.
Root Cause Analysis
There are two core issues in the current implementation:
- Unenforced Lifecycle Management: The framework expects developers to write a
try-finallyblock (as hardcoded inthrowAccessContextNotFoundException). If a developer forgets this, or if an unexpected exception bypasses thefinallyblock, the sensitiveAccessContextremains attached to the worker thread. - Improper ThreadLocal Cleanup: When
AccessContext.clearAccessContextOnThread()is called, it triggers_defaultHolder.save(null), which executes_defaultThreadLocal.set(null). It does not call_defaultThreadLocal.remove(). Setting the value to null leaves a stale Entry in the underlyingThreadLocalMap.
Impact
In environments utilizing thread pools (e.g., Tomcat, Undertow, or RPC workers), threads are persistent and reused. If a thread processing an "Admin" user request retains its AccessContext due to an unhandled exception, that same thread might be assigned to a new, unauthenticated user's request. DBFlute will then erroneously read the residual "Admin" context. This leads to:
- Security Risk: Unauthorized database operations executing with elevated privileges.
- Data Corruption: Audit logs and auto-setup columns recording the wrong user/module.
Suggested Fixes
- Fix the Teardown Logic: Modify
_defaultHolderto explicitly callremove()when the context is cleared.
protected static final AccessContextHolder _defaultHolder = new AccessContextHolder() {
public void save(AccessContext context) {
if (context == null) {
_defaultThreadLocal.remove(); // Explicitly remove instead of set(null)
} else {
_defaultThreadLocal.set(context);
}
}
};