Potential Buffer Overflow Vulnerability in Message Parsing

[Hitachia]

Hi there,

I have identified a potential buffer overflow vulnerability in the TJPConcurrentNetworkManager and TJPNetworkManagerV1 classes related to network message parsing. This issue arises from the lack of validation for the body length of incoming messages, which could lead to excessive memory allocation and potential crashes or security vulnerabilities.

The network message parsing code in both TJPNetworkManagerV1 and TJPConcurrentNetworkManager lacks proper validation for message body length, which creates a potential security vulnerability. An attacker could craft a malicious packet with a very large body length value, causing excessive memory allocation and potential buffer overflow.

Steps to Reproduce

1. Connect to a compromised or simulated malicious server
2. Send a custom TCP packet with a valid header magic number but an extremely large body length value
3. Observe the app crash or experience memory-related issues

Current Behavior

When parsing a message, the code reads the body length from the header and allocates memory accordingly without validating that the length is reasonable. This enables a potential denial-of-service attack or even remote code execution in worst-case scenarios.

Expected Behavior

The code should validate the body length against reasonable maximum limits before allocating memory or attempting to read from the buffer.

Code References

// In TJPConcurrentNetworkManager.m:
- (void)tryParseNextMessageIfNeeded {
    while (true) {
        // ...
        
        //reading message body
        uint32_t bodyLength = ntohl(self->_currentHeader.bodyLength);
        // No validation of bodyLength before memory operations
        if (self.parseBuffer.length < bodyLength) return;
        
        //处理消息体
        NSData *payload = [self.parseBuffer subdataWithRange:NSMakeRange(0, bodyLength)];
        [self.parseBuffer replaceBytesInRange:NSMakeRange(0, bodyLength) withBytes:NULL length:0];
        
        // ...
    }
}

Impact

• Security vulnerability allowing denial-of-service attacks
• Potential memory corruption or buffer overflow
• App crashes under malicious network conditions
• Possible data leakage or remote code execution in extreme cases

Proposed Solution

Add proper validation for the message body length:

- (void)tryParseNextMessageIfNeeded {
    // Define reasonable message size limits
    const uint32_t kMaxAllowedMessageSize = 10 * 1024 * 1024; // 10MB max message size
    
    while (true) {
        // ...existing header parsing code...
        
        uint32_t bodyLength = ntohl(self->_currentHeader.bodyLength);
        
        // Validate body length before proceeding
        if (bodyLength > kMaxAllowedMessageSize) {
            TJPLOG_ERROR(@"安全警告: 消息体长度超过限制 (%u > %u)", bodyLength, kMaxAllowedMessageSize);
            [self handleInvalidData];
            [self resetParse];
            return;
        }
        
        if (self.parseBuffer.length < bodyLength) return;
        
        // Proceed with valid message body processing
        // ...
    }
}

Additionally, implement proper error logging and security event tracking for potential attack detection:

- (void)handleInvalidData {
    // Log security events for monitoring
    NSString *securityLogMessage = [NSString stringWithFormat:@"检测到潜在的安全漏洞攻击: %@", [NSDate date]]; 
    [self logSecurityEvent:securityLogMessage];
    
    // Existing invalid data handling
    NSError *error = [TJPNETError errorWithCode:TJPNETErrorInvalidProtocol
                                      userInfo:@{NSLocalizedDescriptionKey: @"无效协议数据"}];
    [TJPNETErrorHandler handleError:error inManager:self];
    
    // Reset connection to protect the app
    [self resetConnection];
}

- (void)logSecurityEvent:(NSString *)message {
    // Implement security logging for incident response
    TJPLOG_ERROR(@"[安全事件] %@", message);
    // Could add additional security event tracking here
}

These changes would protect the application from potential malicious packets while maintaining compatibility with the existing protocol.