robbiehanson / CocoaAsyncSocket

Asynchronous socket networking library for Mac and iOS
Other
12.45k stars 2.99k forks source link

Buffer overflow #840

Open spraveenk91 opened 2 weeks ago

spraveenk91 commented 2 weeks ago

Potential buffer overflow issue can happen:

NSUInteger cipherIndex;
for (cipherIndex = 0; cipherIndex < numberCiphers; cipherIndex++)
{
    NSNumber *cipherObject = [cipherSuites objectAtIndex:cipherIndex];
    ciphers[cipherIndex] = (SSLCipherSuite)[cipherObject unsignedIntValue];
}
seanm commented 2 weeks ago

How so?

spraveenk91 commented 2 weeks ago

@seanm We have scanned the library with HP Fortify (Secure Code Analysis) tool and we have got this:

The function ssl_startTLS() in GCDAsyncSocket.m writes outside the bounds of ciphers on line 7142, which could corrupt data, cause the program to crash, or lead to the execution of malicious code.

It has also given some recommendations to consider:

Never use inherently unsafe functions, such as gets(), and avoid the use of functions that are difficult to use safely such as strcpy(). Replace unbounded functions like strcpy() with their bounded equivalents, such as strncpy() or the WinAPI functions defined in strsafe.h . Although the careful use of bounded functions can greatly reduce the risk of buffer overflow, this migration cannot be done blindly and does not go far enough on its own to ensure security. Whenever you manipulate memory, especially strings, remember that buffer overflow vulnerabilities typically occur in code that:

  • Relies on external data to control its behavior
  • Depends upon properties of the data that are enforced outside of the immediate scope of the code
  • Is so complex that a programmer cannot accurately predict its behavior. Additionally, consider the following principles:
  • Never trust an external source to provide correct control information to a memory operation.
  • Never trust that properties about the data your program is manipulating will be maintained throughout the program. Sanity check data before you operate on it.
  • Limit the complexity of memory manipulation and bounds-checking code. Keep it simple and clearly document the checks you perform, the assumptions that you test, and what the expected behavior of the program is in the case that input validation fails.
  • When input data is too large, be leery of truncating the data and continuing to process it. Truncation can change the meaning of the input.
  • Do not rely on tools, such as StackGuard, or non-executable stacks to prevent buffer overflow vulnerabilities. These approaches do not address heap buffer overflows and the more subtle stack overflows that can change the contents of variables that control the program. Additionally, many of these approaches are easily defeated, and even when they are working properly, they address the symptom of the problem and not its cause.
seanm commented 2 weeks ago

I had no idea that tool supported Objective-C. Have you found it a useful tool for Obj-C?

Static analysis often has false positives, this looks like one to me. Have you reasoned otherwise?