[
https://issues.apache.org/jira/browse/LANG-1799?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gary D. Gregory updated LANG-1799:
----------------------------------
Summary: Make private static variables final in RandomStringUtils (was:
Non-final Static Singleton Variables Break Thread Safety and Singleton Design
in RandomStringUtils)
> Make private static variables final in RandomStringUtils
> --------------------------------------------------------
>
> Key: LANG-1799
> URL: https://issues.apache.org/jira/browse/LANG-1799
> Project: Commons Lang
> Issue Type: Bug
> Components: lang.*
> Affects Versions: 3.20.0
> Reporter: Zhongxin Yan
> Priority: Major
>
> The RandomStringUtils class exposes three primary singleton instances
> (INSECURE, SECURE, SECURE_STRONG) through static fields that are currently
> missing the final keyword.
>
> {code:java}
> private static RandomStringUtils INSECURE = new
> RandomStringUtils(RandomUtils::insecure);
> private static RandomStringUtils SECURE = new
> RandomStringUtils(SECURE_SUPPLIER);
> private static RandomStringUtils SECURE_STRONG = new
> RandomStringUtils(RandomUtils::secureStrong);
> {code}
> h3. Key Issues Caused by Non-final Modifier:
> * {*}Unintended Reassignment{*}: The variables can be modified at runtime
> (e.g., via reflection, accidental code changes, or subclassing), breaking the
> singleton contract and leading to inconsistent random string generation
> behavior across the application.
> * {*}Violation of Documentation{*}: The class Javadoc explicitly labels it
> {{{}#ThreadSafe#{}}}, but non-final static state undermines this promise.
>
> {code:java}
> // Test
> /**
> * Test case to demonstrate the security and robustness risk introduced
> * by the non-final static singleton fields in RandomStringUtils.
> * * NOTE: This test should fail when the code is fixed (i.e., when fields
> are made final).
> */
> public class RandomStringUtilsNonFinalSingletonTest {
> private static final String FIELD_NAME = "SECURE";
> /**
> * Attempts to corrupt the static singleton reference via reflection.
> */
> @Test
> void testNonFinalStaticSingletonCorruption() throws Exception {
> // 1. Get the non-final static field
> Field secureField =
> RandomStringUtils.class.getDeclaredField(FIELD_NAME);
> // Ensure the field is NOT final (pre-condition for the bug)
>
> assertFalse(java.lang.reflect.Modifier.isFinal(secureField.getModifiers()),
> "Pre-condition failed: Field should not be final to
> demonstrate the corruption risk.");
> // 2. Override accessibility for private field access
> secureField.setAccessible(true);
> // 3. Store the original instance for cleanup
> RandomStringUtils originalInstance = (RandomStringUtils)
> secureField.get(null);
> assertNotNull(originalInstance, "Original singleton must be
> initialized.");
> try {
> // 4. CORRUPT THE SINGLETON REFERENCE ***
> // Setting a static field: pass null as the first argument
> secureField.set(null, null);
> // 5. Verify corruption succeeded
> assertNull(secureField.get(null), "Non-final static field was
> successfully set to null.");
> // 6. Assert that subsequent application logic fails with NPE
> // This proves the integrity of the singleton has been
> compromised.
> assertThrows(NullPointerException.class, () -> {
> // The call to RandomStringUtils.secure() returns null, and
> .next(1) fails.
> RandomStringUtils.secure().next(1);
> }, "Subsequent calls to the corrupted singleton must throw NPE.");
> } finally {
> // Cleanup: Restore the original instance to avoid breaking other
> tests
> if (secureField.get(null) == null) {
> secureField.set(null, originalInstance);
> }
> }
> }
> }{code}
> [github pr|https://github.com/apache/commons-lang/pull/1513]
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
