-
Notifications
You must be signed in to change notification settings - Fork 816
Description
Bug Description
In org.apache.poi.hssf.record.crypto.Biff8EncryptionKey, a static ThreadLocal<String> _userPasswordTLS is used to store the BIFF8 encryption/decryption password. As explicitly acknowledged in the Javadoc, this was implemented as a design compromise to avoid "further overloading the various public APIs".
While this approach works in single-threaded or desktop applications, it introduces a severe Credential Leak Vulnerability (Type I Security Risk) when Apache POI is integrated into modern, thread-pool-based web server environments (e.g., Spring Boot, Tomcat).
Root Cause & Reproduction Scenario
Web containers use persistent, pooled worker threads. When an application sets a password to read an encrypted Excel file but fails to explicitly clear it (which is extremely common since there is no forced try-with-resources or finally enforcement at the API level), the clear-text password becomes a "zombie" object residing indefinitely in the thread's ThreadLocalMap.
// Typical vulnerable usage in a Spring Boot Controller
@PostMapping("/upload-encrypted-xls")
public void processExcel(@RequestParam String password, @RequestParam MultipartFile file) {
// 1. Password set on the Tomcat Worker Thread
Biff8EncryptionKey.setCurrentUserPassword(password);
// 2. Process the file...
HSSFWorkbook workbook = new HSSFWorkbook(file.getInputStream());
// 3. If an exception occurs here, or the developer simply forgets to call:
// Biff8EncryptionKey.setCurrentUserPassword(null);
// The password permanently leaks into the ThreadLocalMap of this Tomcat worker thread.
}
Security Impact
1. Memory Residency: A history of user passwords will gradually accumulate and persist in the server's heap memory. Any heap dump (OOM generated or malicious) will expose these clear-text passwords.
2. String Immutability: Because the password is stored as an immutable String rather than a char[], it cannot be overwritten in memory even if remove() is called, keeping it vulnerable until unpredictable Garbage Collection occurs.
3. Cross-Request Contamination: If a subsequent request is processed by the same tainted thread, it might unintentionally gain access to the decrypted contents of a file without the user explicitly providing the password.
Suggested Fixes
Since fundamentally changing the API signatures across HSSFWorkbook is highly disruptive, consider introducing a structured concurrency approach (Closure/Scope) to enforce lifecycle safety without altering existing signatures:
1. Introduce a try-with-resources Scope:
public static AutoCloseable withPassword(String password) {
setCurrentUserPassword(password);
return () -> setCurrentUserPassword(null); // Or better, _userPasswordTLS.remove()
}
Usage:
try (AutoCloseable ignored = Biff8EncryptionKey.withPassword(password)) {
HSSFWorkbook workbook = new HSSFWorkbook(stream);
} // Safely and automatically removed here