We’ve improved the C++ Code Analysis toolset with every major compiler update in Visual Studio 2017. Version 15.6, now in Preview, includes a set of arithmetic overflow checks. This article discusses those checks and why you’ll want to enable them in your code.
If you’re just getting started with C++ Code Analysis in Visual Studio, learn more about the overall experience in this overview blog post.
Motivation
As part of the C++ Code Analysis team, we work with groups across the company to identify classes of vulnerabilities that could be detected with better tooling and language support. Recently, as part of a security review in one of Microsoft’s most security sensitive codebase, we found we needed to add checks for detecting a common class of arithmetic overflows.
Example patterns:
uint32 ConvertOffset(uint32 ptr, uint32 fromSize, uint32 toSize) { if (fromSize == toSize) { return ptr; } uint64 tmp = ptr * fromSize; // “tmp” can overflow here if ptr * fromSize is wider than uint32 // More code } template class BucketEntryIterator sealed : public IteratorBase<TDictionary, BucketEntryIterator> { private: uint bucketIndex; // Rest of the data members public: BucketEntryIterator(TDictionary &dictionary) : Base(dictionary, 1), bucketIndex(0u – 1) // This wraps past 0 to a really big number, which can overflow bucketIndex // Initialize other data members of the class };
We looked into the C++ Core Guidelines to see if there are specific guidelines in this space.
Arithmetic rules
 ES.100: Don’t mix signed and unsigned arithmetic
 ES.101: Use unsigned types for bit manipulation
 ES.102: Use signed types for arithmetic
 ES.103: Don’t overflow
 ES.104: Don’t underflow
 ES.105: Don’t divide by zero
 ES.106: Don’t try to avoid negative values by using unsigned
 ES.107: Don’t use unsigned for subscripts, prefer gsl::index
The guidelines note that some of these rules can be tricky to enforce in practice. Therefore, we took an approach where we tried to intersect the set of guidelines with the kind of defect patterns we wanted to detect in our implementation.
Checks
The following are a set of arithmetic checks we added to C++ Core Check for 15.6 release:

C26450 RESULT_OF_ARITHMETIC_OPERATION_PROVABLY_LOSSY[operator] operation causes overflow at compile time. Use a wider type to store the operands.
This warning indicates that an arithmetic operation was provably lossy at compile time. This can be asserted when the operands are all compiletime constants. Currently, we check left shift, multiplication, addition, and subtraction operations for such overflows.
// Example source: int multiply() { const int a = INT_MAX; const int b = 2; int c = a * b; // C26450 reported here return c; }
// Corrected source: long long multiply() { const int a = INT_MAX; const int b = 2; long long c = (long long)a * b; // OK return c; }
In the corrected source, the left operand was cast to a wider type for the result of the arithmetic operation to be wider.

C26451 RESULT_OF_ARITHMETIC_OPERATION_CAST_TO_LARGER_SIZE Using operator [operator] on a [size1] byte value and then casting the result to a [size2] byte value. Cast the value to the wider type before calling operator [operator] to avoid overflow
This warning indicates incorrect behavior that results from integral promotion rules and types larger than those in which arithmetic is typically performed. We detect when a narrow type integral value was shifted left, multiplied, added, or subtracted and the result of that arithmetic operation was cast to a wider type value. If the operation overflowed the narrow type value, then data is lost. You can prevent this loss by casting the value to a wider type before the arithmetic operation.
// Example source: void leftshift(int i) { unsigned long long x; x = i << 31; // C26451 reported here // code }
// Corrected source: void leftshift(int i) { unsigned long long x; x = (unsigned long long)i << 31; // OK // code }
In the corrected source, the left operand was cast to a wider type for the result of the arithmetic operation to be wider.

C26452 SHIFT_COUNT_NEGATIVE_OR_TOO_BIG Left shift count is negative or greater than or equal to the operand size which is undefined behavior
This warning indicates a shift count is negative or greater than or equal to the number of bits of the operand being shifted, resulting in undefined behavior.
// Example source: unsigned long long combine(unsigned lo, unsigned hi) { return (hi << 32)  lo; // C26252 here }
// Corrected source: unsigned long long combine(unsigned lo, unsigned hi) { return ((unsigned long long)hi << 32)  lo; // OK }
In the corrected source, the left operand was cast to a 64 bit value before left shifting it by 32 bits.

C26453 LEFTSHIFT_NEGATIVE_SIGNED_NUMBER Left shift of a negative signed number is undefined behavior
This warning indicates we are left shifting a negative signed integral value, which is a bad idea and triggers implementation defined behavior.
// Example source: void leftshift(int shiftCount) { const auto result = 1 << shiftCount; // C26453 reported here // code }
// Corrected source: void leftshift(int shiftCount) { const auto result = 4294967295 << shiftCount; // OK // code }
In the corrected source, a positive integral value was used for the shift operation.

C26454 RESULT_OF_ARITHMETIC_OPERATION_NEGATIVE_UNSIGNED [operator] operation wraps past 0 and produces a large unsigned number at compile time
This warning indicates that the subtraction operation produces a negative result which was evaluated in an unsigned context. This causes the result to wrap past 0 and produce a really large unsigned number, which can result in unintended overflows.
// Example source: unsigned int negativeunsigned() { const unsigned int x = 1u  2u; // C26454 reported here return x; }
// Corrected source: unsigned int negativeunsigned() { const unsigned int x = 4294967295; // OK return x; }
In the corrected source, a positive value was assigned to the unsigned result.
Results
We ran these checks in a securitysensitive codebase over the holidays and found interesting bug patterns. I am resharing the example patterns that looked suspicious at the beginning of this blog post along with the code analysis warnings they now trigger.
Warning C26451 Arithmetic overflow: Using operator '*' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '*' to avoid overflow
uint32 ConvertOffset(uint32 ptr, uint32 fromSize, uint32 toSize) { if (fromSize == toSize) { return ptr; } uint64 tmp = ptr * fromSize; // C26451 // More code }
Warning C26454 Arithmetic overflow: '' operation produces a negative unsigned result at compile time, resulting in an overflow
template class BucketEntryIterator sealed : public IteratorBase<TDictionary, BucketEntryIterator> { private: uint bucketIndex; // Rest of the data members public: BucketEntryIterator(TDictionary &dictionary) : Base(dictionary, 1), bucketIndex(0u – 1) // C26454 // Initialize other data members of the class };
Feedback
We’d love for you to try these checks firsthand. Download Version 15.6 Preview 6 and let us know if you find any interesting bug patterns in your codebase.
As always, if you have any feedback or suggestions for us, let us know. We can be reached via the comments below, via email (visualcpp@microsoft.com) and you can provide feedback via Help > Report A Problem in the product, or via Developer Community. You can also find us on Twitter (@VisualC) and Facebook (msftvisualcpp).
Join the conversation
Add Comment