Description
Describe the bug
In the method ParameterList::combineMultisegmentNames(boolean)
method, there is an iteration for the multisegmentNames
set. There is also a statement on line 433 that removes items from the multisegmentNames
set within the loop. If line 433 is invoked, then the next call to the next() method of the set iterator will throw a ConcurrentModificationException.
Although there is a comment on line 432 mentions that the set removes method on line 433 should not be called, it is found that there are some invalid inputs that could pass the checking on line 431 and invoke line 433 which causes ConcurrentModificationException from the next Iterator::next() call on line 408.
Here are more details about the possible bug.
In the constructor of ParameterList, there is a call to the ParameterList::combineMultisegmentNames(boolean)
method on line 309. That parameter takes a random String object to process. If we pass in an invalid string ;'*0=$;*0=Sg';*1*=$
, it can pass all parsing and checking and get to line 309 and invoke the ParameterList::combineMultisegmentNames(boolean)
method. At that moment, the first item in the multisegmentNames
set will be an empty string, and the slist
map will contain three key-value pairs as follows.
Key | Value |
'*0 | $ |
*0 | Sg' |
*1 | An object of ParameterList$Value |
In the first iteration of the segment on line 417, as name
is an empty string, thus the sname
on line 418 will become *0
which makes the Object v on line 419 becomes a String object of $
. As the Object v is not an instance of ParameterList$Value, thus the conditional checking on line 424 will be false and jump to 438, this makes the charset
variable stay null.
Then in the next segment iteration, the sname
on line 418 will become *1
which makes the Object v on line 419 becomes an instance of the ParameterList$Value. With the newest Object v, the conditional checking on line 424 is true and gets to the inner logic, but because the segment is already 1 for this iteration, it will jump to another conditional checking on line 431. Because the charset
variable is still null, the condition is met and thus the multisegmentNames
is modified and causes ConcurrentModificationException in the next call to Iterator::next() method on line 408 during the next multisegmentNames
iteration.
To Reproduce
Here is a sample proof of concept code.
import jakarta.mail.internet.ParameterList;
public class ProofOfConcept {
public static void main(String...args) throws Exception {
new ParameterList(";'*0=$;*0=Sg';*1*=$");
}
}
Just compile and run it will directly cause the ConcurrentModificationException.
Expected behaviour
ParseException should be thrown for invalid input instead of just a comment mentioning that it should not happen.