Buggy Security Guidance from Apple
The document is on the right track to a solution. The key is to be able to detect the overflow situation without triggering it. Or in this specific case, detect that n * m would overflow, without actually calculating the value of bytes. But putting the detection after the calculation of bytes defeats that purpose because by then we’ve triggered undefined behavior. We need to use the detection to avoid the calculation that would result in undefined behavior.
Some optimizing compilers exploit undefined behavior by using it to prune the state tree. Since signed integer overflow is undefined the compiler is allowed to assume that it cannot happen. Therefore, at the time of the ‘if’ statement the compiler is allowed to assume that n*m does not overflow, and the compiler may therefore determine that the overflow checks will always pass, and can therefore be removed.
[…]
At some point Apple quietly fixed their document. They didn’t change the date (the footer still says February 11 but the properties now say March 10) or acknowledge the error (the revision history is silent), but they fixed the code.
[…]
They now avoid doing the multiplication until they have done the check, but their code is still wrong.
SIZE_MAX is the maximum value of a size_t typed variable. We’ve already established that ‘n’ and ‘m’ are not size_t types – they appear to be signed integers. The maximum value for a signed integer is INT_MAX. Apple is using the wrong constant!
