Ruiqi Dong created CLI-348:
------------------------------
Summary: 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
Reporter: Ruiqi Dong
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)
