Infinite Loop in Network Message Parsing

[don3ki]

The TJPConcurrentNetworkManager implementation contains a critical bug in its message parsing logic. The tryParseNextMessageIfNeeded method uses a while(true) loop without a proper exit condition, which can lead to an infinite loop when specific data conditions are met.

Steps to Reproduce

1. Connect to a server using the TJPConcurrentNetworkManager
2. Receive malformed data that passes the magic number check but has invalid header information
3. Observe the app becoming unresponsive

Current Behavior

When parsing network messages in TJPConcurrentNetworkManager.m, the method uses a while(true) loop but only returns from inside conditional blocks. If certain conditions are met where the header is valid but the data is malformed, the loop will continue indefinitely, blocking the network queue and potentially making the app unresponsive.

- (void)tryParseNextMessageIfNeeded {
    while (true) {  // Infinite loop with no break condition
        // Logic with return statements in conditionals
        // But no break statement or other exit condition
    }
}

Expected Behavior

The loop should have a proper exit condition or use a break statement to avoid infinite looping. It should also include logic to handle potentially malformed data that passes initial validation.

Code References

// In TJPConcurrentNetworkManager.m:
- (void)tryParseNextMessageIfNeeded {
    while (true) {
        if (self->_isParsingHeader) {
            if (self.parseBuffer.length < sizeof(TJPAdavancedHeader)) return;
            // ...more code
        }
        
        //reading message body
        uint32_t bodyLength = ntohl(self->_currentHeader.bodyLength);
        if (self.parseBuffer.length < bodyLength) return;
        
        // ...more code
        self->_isParsingHeader = YES;
    }
}

Impact

• App becomes unresponsive when certain network conditions occur
• CPU usage spikes to 100%
• Potential battery drain and overheating
• App may need to be force-quit by the user

Proposed Solution

Replace the while(true) loop with a more robust implementation that includes a maximum iteration count and proper loop exit:

- (void)tryParseNextMessageIfNeeded {
    // Set a reasonable max iterations to prevent infinite loop
    const int MAX_ITERATIONS = 100;
    int iterations = 0;
    
    // Process available data up to a reasonable limit
    while (iterations < MAX_ITERATIONS && self.parseBuffer.length > 0) {
        iterations++;
        
        if (self->_isParsingHeader) {
            if (self.parseBuffer.length < sizeof(TJPAdavancedHeader)) break;
            // Existing header parsing logic
        }
        
        uint32_t bodyLength = ntohl(self->_currentHeader.bodyLength);
        
        // Sanity check for body length to prevent overflow or malicious packets
        if (bodyLength > kMaxAllowedMessageSize) {
            [self handleInvalidData];
            [self resetParse];
            break;
        }
        
        if (self.parseBuffer.length < bodyLength) break;
        
        // Continue with existing message processing logic
        // ...
        
        self->_isParsingHeader = YES;
    }
    
    // Log a warning if we hit the iteration limit
    if (iterations >= MAX_ITERATIONS) {
        TJPLOG_WARN(@"Message parsing hit iteration limit, possible malformed data");
    }
}

This change would prevent infinite loops while maintaining functionality for normal cases.