Security Finding: Missing Braces Logic Error leading to UI Denial of Service

Severity: MEDIUM
Reported by: FailSafe Research Team
Component: modules/impl/gui/src/main/java/net/java/sip/communicator/impl/gui/main/chat/conference/ConferenceChatSession.java:437

Description

In ConferenceChatSession.java, the memberPresenceChanged method contains an if statement that checks if a contact is already in the chatParticipants list before adding it. However, the if statement lacks curly braces, meaning only the chatParticipants.add(chatContact); line is conditionally executed. The subsequent line, sessionRenderer.addChatContact(chatContact);, is executed unconditionally for every MEMBER_JOINED event. If a malicious server or user sends continuous MEMBER_JOINED events for the same user, the application will repeatedly add the contact to the UI renderer, consuming excessive CPU and memory.

 432                // Check if not ever present in the chat room. In some cases, the
 433                // considered chatroom member may appear twice in the chat contact
 434                // list panel.
 435                synchronized (chatParticipants)
 436                {
 437>>>                 if (!chatParticipants.contains(chatContact))
 438                        chatParticipants.add(chatContact);
 439                        sessionRenderer.addChatContact(chatContact);
 440                }
 441                
 442                ChatRoom room = chatRoomWrapper.getChatRoom();

Fix

Add curly braces to the if statement to ensure both the list addition and the UI rendering are conditionally executed only when the contact is not already present.

if (!chatParticipants.contains(chatContact)) {
 chatParticipants.add(chatContact);
 sessionRenderer.addChatContact(chatContact);
}

This ensures the UI is only updated when a genuinely new participant joins the chat, preventing the resource exhaustion.

Proof of Concept

1. Analyze the logic error in ConferenceChatSession.java. In Java, an 'if' statement without braces only applies to the immediately following statement.
2. Observe that 'chatParticipants.add(chatContact);' is conditional, but 'sessionRenderer.addChatContact(chatContact);' executes unconditionally for every event.
3. Remediate the vulnerability by enclosing both statements within the 'if' block using curly braces.