David LeBlanc and I have written a good deal about Integer Overflow issues, including the following:
- WSC 2nd Ed: pp620-624.
- Reviewing Code for Integer Manipulation Vulnerabilities (http://msdn.microsoft.com/library/en-us/dncode/html/secure04102003.asp)
- Integer Handling with the C++ SafeInt Class (http://msdn.microsoft.com/library/en-us/dncode/html/secure01142004.asp)
- An Overlooked Construct and an Integer Overflow Redux (http://msdn.microsoft.com/library/en-us/dncode/html/secure09112003.asp)
A couple of days ago I saw some code from someone outside of Microsoft claiming they had found a new (read: cheap) way to detect integer overflow errors, here's the code snippet:
void *p= NULL;
size_t cb = z + (x * y);
if ((int)cb > 0 && cb < MAX)
Basically, you cast the result to signed, and if it’s negative, then there must be an overflow… right?
I had no spare cycles, so I asked David to look at it. He shot the code down in about 15secs. So what's wrong with the code?