[
https://issues.apache.org/jira/browse/CLI-348?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ruiqi Dong updated CLI-348:
---------------------------
Affects Version/s: 1.10.0
> MissingOptionException lacks defensive copying, allowing external
> modification of internal state
> ------------------------------------------------------------------------------------------------
>
> Key: CLI-348
> URL: https://issues.apache.org/jira/browse/CLI-348
> Project: Commons CLI
> Issue Type: Bug
> Affects Versions: 1.10.0
> Reporter: Ruiqi Dong
> Priority: Major
>
> The {{MissingOptionException}} class has a defensive copying vulnerability
> that allows external code to modify the exception's internal state after
> construction. This violates the principle of encapsulation and can lead to
> unpredictable behavior, especially in multi-threaded environments. As the
> constructor {{MissingOptionException(List missingOptions)}} directly stores
> the reference to the passed list without creating a copy, the getter method
> {{getMissingOptions()}} returns a direct reference to the internal list,
> allowing external modification.
>
> *Impact:*
> * Violates the principle of immutability for exception objects
> * Thread safety issues in concurrent environments
> * Potential security vulnerability allowing unauthorized state modification
> * Breaks the contract that exceptions should represent a fixed error state
> *Test Case:*
> {code:java}
> @Test
> void testDefensiveCopyAndImmutabilityOfMissingOptions() {
> List<String> originalList = new ArrayList<>(Arrays.asList("optA",
> "optB"));
> MissingOptionException exception = new
> MissingOptionException(originalList); originalList.add("optC");
> originalList.remove(0); assertEquals(Arrays.asList("optA", "optB"),
> exception.getMissingOptions(),
> "Missing options list should be isolated from original list
> modifications"); List<String> retrievedList =
> exception.getMissingOptions();
> assertThrows(UnsupportedOperationException.class, () ->
> retrievedList.add("optD"),
> "Missing options list should prevent modifications through defensive
> immutability");
> } {code}
> *Failure Scenario:*
> {code:java}
> [ERROR]
> org.apache.commons.cli.MissingOptionExceptionTest.testDefensiveCopyAndImmutabilityOfMissingOptions
> -- Time elapsed: 0.001 s <<< FAILURE!
> org.opentest4j.AssertionFailedError: Missing options list should be isolated
> from original list modifications ==> expected: <[optA, optB]> but was:
> <[optB, optC]>
> at
> org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
> at
> org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
> at
> org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
> at
> org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
> at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1156)
> at
> org.apache.commons.cli.MissingOptionExceptionTest.testDefensiveCopyAndImmutabilityOfMissingOptions(MissingOptionExceptionTest.java:164)
> at java.lang.reflect.Method.invoke(Method.java:498)
> at java.util.ArrayList.forEach(ArrayList.java:1259)
> at java.util.ArrayList.forEach(ArrayList.java:1259) {code}
> *Suggested Fix:*
> {code:java}
> public MissingOptionException(final List missingOptions) {
> super(createMessage(missingOptions));
> this.missingOptions = missingOptions == null ? null :
> Collections.unmodifiableList(new ArrayList<>(missingOptions));
> }{code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
