Skip to content

[BUG] Potential Tenant Identity Leak and Data Cross-Pollination via InheritableThreadLocal in TenantContext #13

Description

@QiuYucheng2003

Describe the bug
A severe Type III: Context Pollution vulnerability exists in com.example.config.TenantContext. The implementation uses InheritableThreadLocal to store tenant IDs but lacks proper cleanup and safe handling for thread-reusing environments.

Specific Issues:

  1. Pseudo-Cleanup: The clear() method calls .set(null) instead of .remove(). This leaves stale entries in the ThreadLocalMap, preventing memory reclamation and potentially leaking internal metadata.

  2. Context Bleeding: The use of InheritableThreadLocal causes tenant IDs to automatically propagate to child threads. In environments where threads are reused (e.g., Spring @async or CompletableFuture pools), this leads to Identity Pollution, where a thread incorrectly inherits a tenant context from a previously processed, unrelated task.

Impact:
This poses a high risk of unauthorized data access. A request for Tenant B could be processed using a leaked context from Tenant A, leading to data leaks or corrupted audit logs.

How to reproduce?
This was identified via static analysis. The failure path is as follows:

  1. Thread-1 handles a request for Tenant A and spawns a child task (inheriting the ID).

  2. Thread-1 finishes but fails to call remove() (or only sets to null).

  3. Thread-1 is returned to the pool and reused for Tenant B.

  4. Due to the inheritance and lack of mandatory removal, the thread may still resolve to Tenant A's context in certain async boundary scenarios, causing cross-tenant data access.
    // Conceptual reproduction of context bleed:
    TenantContext.setCurrentTenant("Tenant-A");
    // ... child thread spawned ...
    TenantContext.clear(); // only sets null, entry still exists

// Later, on a reused thread from a pool
String leakedTenant = TenantContext.getCurrentTenant();
// Expected: null or new tenant | Actual: Risk of stale "Tenant-A" inheritance

Suggested Fix
To resolve the memory leak and mitigate context pollution in async environments, the following refactoring is recommended:

  1. Use .remove() instead of .set(null)
    Explicitly calling remove() ensures that the ThreadLocalMap entry is purged, allowing the GC to reclaim the data and the entry object.

  2. Downgrade to standard ThreadLocal
    Unless there is a strict requirement for automatic child-thread inheritance, use a standard ThreadLocal to prevent uncontrolled context propagation.

Revised Code Snippet:
package com.example.config;

public class TenantContext {
// Recommendation 1: Use ThreadLocal to avoid async pool pollution
private static final ThreadLocal currentTenant = new ThreadLocal<>();

public static String getCurrentTenant() {
    return currentTenant.get();
}

public static void setCurrentTenant(String tenant) {
    currentTenant.set(tenant);
}

public static void clear() {
    // Recommendation 2: Use remove() for complete cleanup
    currentTenant.remove();
}

}

  1. Enforce Framework-Level Cleanup
    Implement a Web Interceptor or Filter to ensure TenantContext.clear() is called in a finally block (or afterCompletion) for every request, ensuring threads are clean before returning to the pool.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions