Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible ConcurrentModificationException in jakarta.mail.internet.ParameterList #734

Open
arthurscchan opened this issue Jul 11, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@arthurscchan
Copy link

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.

KeyValue
'*0$
*0Sg'
*1An 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants