{"id":18306,"date":"2018-02-23T14:09:15","date_gmt":"2018-02-23T22:09:15","guid":{"rendered":"https:\/\/blogs.msdn.microsoft.com\/vcblog\/?p=18306"},"modified":"2019-02-18T17:48:03","modified_gmt":"2019-02-18T17:48:03","slug":"arithmetic-overflow-checks-in-c-core-check","status":"publish","type":"post","link":"https:\/\/devblogs.microsoft.com\/cppblog\/arithmetic-overflow-checks-in-c-core-check\/","title":{"rendered":"Arithmetic overflow checks in C++ Core Check"},"content":{"rendered":"<p><a href=\"https:\/\/blogs.msdn.microsoft.com\/c\/2018\/02\/26\/c-%e6%a0%b8%e5%bf%83%e6%a3%80%e6%9f%a5%e4%b8%ad%e7%9a%84%e7%ae%97%e6%9c%af%e6%ba%a2%e5%87%ba%e6%a3%80%e6%9f%a5\/\">\u70b9\u8fd9\u91cc\u770b\u4e2d\u6587\u7248<\/a><\/p>\n<p>We&#8217;ve improved the C++ Code Analysis toolset with every major compiler update in Visual Studio 2017. <a href=\"https:\/\/www.visualstudio.com\/vs\/preview\/\">Version 15.6, now in Preview<\/a>, includes a set of arithmetic overflow checks. This article discusses those checks and why you\u2019ll want to enable them in your code.<\/p>\n<p>If you\u2019re just getting started with C++ Code Analysis in Visual Studio, learn more about the overall experience <a href=\"https:\/\/blogs.msdn.microsoft.com\/vcblog\/2018\/01\/10\/c-static-analysis-improvements-for-visual-studio-2017-15-6-preview-2\/\">in this overview blog post<\/a>.<\/p>\n<h3>Motivation<\/h3>\n<p>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\u2019s most security sensitive codebase, we found we needed to add checks for detecting a common class of arithmetic overflows.<\/p>\n<p>Example patterns:<\/p>\n<pre class=\"prettyprint\">\nuint32 ConvertOffset(uint32 ptr, uint32 fromSize, uint32 toSize)\n{\n    if (fromSize == toSize)\n    {\n        return ptr;\n    }\n    uint64 tmp = ptr * fromSize; \/\/ \u201ctmp\u201d can overflow here if ptr * fromSize is wider than uint32\n    \n    \/\/ More code\n}\n\ntemplate\nclass BucketEntryIterator sealed : public IteratorBase&amp;lt;TDictionary, BucketEntryIterator&amp;gt;\n{\nprivate:\n    uint bucketIndex;\n    \/\/ Rest of the data members\n\npublic:\n    BucketEntryIterator(TDictionary &amp;amp;dictionary)\n    : Base(dictionary, -1),\n      bucketIndex(0u \u2013 1) \/\/ This wraps past 0 to a really big number, which can overflow bucketIndex\n      \n      \/\/ Initialize other data members of the class\n};\n<\/pre>\n<p>We looked into the <a href=\"https:\/\/github.com\/isocpp\/CppCoreGuidelines\">C++ Core Guidelines<\/a> to see if there are specific guidelines in this space.<\/p>\n<h4>Arithmetic rules<\/h4>\n<ul>\n<li><a href=\"https:\/\/github.com\/isocpp\/CppCoreGuidelines\/blob\/master\/CppCoreGuidelines.md#Res-mix\">ES.100: Don\u2019t mix signed and unsigned arithmetic<\/a><\/li>\n<li><a href=\"https:\/\/github.com\/isocpp\/CppCoreGuidelines\/blob\/master\/CppCoreGuidelines.md#Res-unsigned\">ES.101: Use unsigned types for bit manipulation<\/a><\/li>\n<li><a href=\"https:\/\/github.com\/isocpp\/CppCoreGuidelines\/blob\/master\/CppCoreGuidelines.md#Res-signed\">ES.102: Use signed types for arithmetic<\/a><\/li>\n<li><a href=\"https:\/\/github.com\/isocpp\/CppCoreGuidelines\/blob\/master\/CppCoreGuidelines.md#Res-overflow\">ES.103: Don\u2019t overflow<\/a><\/li>\n<li><a href=\"https:\/\/github.com\/isocpp\/CppCoreGuidelines\/blob\/master\/CppCoreGuidelines.md#Res-underflow\">ES.104: Don\u2019t underflow<\/a><\/li>\n<li><a href=\"https:\/\/github.com\/isocpp\/CppCoreGuidelines\/blob\/master\/CppCoreGuidelines.md#Res-zero\">ES.105: Don\u2019t divide by zero<\/a><\/li>\n<li><a href=\"https:\/\/github.com\/isocpp\/CppCoreGuidelines\/blob\/master\/CppCoreGuidelines.md#Res-nonnegative\">ES.106: Don\u2019t try to avoid negative values by using unsigned<\/a><\/li>\n<li><a href=\"https:\/\/github.com\/isocpp\/CppCoreGuidelines\/blob\/master\/CppCoreGuidelines.md#Res-subscripts\">ES.107: Don\u2019t use unsigned for subscripts, prefer gsl::index<\/a><\/li>\n<\/ul>\n<p>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\u00a0of defect patterns we wanted to detect in our implementation.<\/p>\n<h3>Checks<\/h3>\n<p>The following are\u00a0a set of arithmetic checks we added to C++ Core Check for 15.6 release:<\/p>\n<ul>\n<li><a href=\"https:\/\/docs.microsoft.com\/visualstudio\/code-quality\/c26450\">C26450 RESULT_OF_ARITHMETIC_OPERATION_PROVABLY_LOSSY<\/a>[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 compile-time constants. Currently, we check left shift, multiplication, addition, and subtraction operations for such overflows.\n<pre class=\"prettyprint\">\n\/\/ Example source:\nint multiply() {\n    const int a = INT_MAX;\n    const int b = 2;\n    int c = a * b; \/\/ C26450 reported here\n    return c;\n}\n<\/pre>\n<pre class=\"prettyprint\">\n\/\/ Corrected source:\nlong long multiply() {\n    const int a = INT_MAX;\n    const int b = 2;\n    long long c = (long long)a * b; \/\/ OK\n    return c;\n}\n<\/pre>\n<p>In the corrected source, the left operand was cast to a wider type for the result of the arithmetic operation to be wider.<\/li>\n<li><a href=\"https:\/\/docs.microsoft.com\/visualstudio\/code-quality\/c26451\">C26451 RESULT_OF_ARITHMETIC_OPERATION_CAST_TO_LARGER_SIZE<\/a> 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 overflowThis 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.\n<pre class=\"prettyprint\">\n\/\/ Example source:\nvoid leftshift(int i) {\n    unsigned long long x;\n    x = i &lt;&lt; 31; \/\/ C26451 reported here\n         \/\/ code\n}\n<\/pre>\n<pre class=\"prettyprint\">\n\/\/ Corrected source:\nvoid leftshift(int i) {\n    unsigned long long x;\n    x = (unsigned long long)i &lt;&lt; 31; \/\/ OK\n        \/\/ code\n}\n<\/pre>\n<p>In the corrected source, the left operand was cast to a wider type for the result of the arithmetic operation to be wider.<\/li>\n<li><a href=\"https:\/\/docs.microsoft.com\/visualstudio\/code-quality\/c26452\">C26452 SHIFT_COUNT_NEGATIVE_OR_TOO_BIG<\/a> Left shift count is negative or greater than or equal to the operand size which is undefined behaviorThis 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.\n<pre class=\"prettyprint\">\n\/\/ Example source:\nunsigned long long combine(unsigned lo, unsigned hi)\n{\n    return (hi &lt;&lt; 32) | lo; \/\/ C26252 here\n}\n<\/pre>\n<pre class=\"prettyprint\">\n\/\/ Corrected source:\nunsigned long long combine(unsigned lo, unsigned hi)\n{\n    return ((unsigned long long)hi &lt;&lt; 32) | lo; \/\/ OK\n}\n<\/pre>\n<p>In the corrected source, the left operand was cast to a 64 bit value before left shifting it by 32 bits.<\/li>\n<li><a href=\"https:\/\/docs.microsoft.com\/visualstudio\/code-quality\/c26453\">C26453 LEFTSHIFT_NEGATIVE_SIGNED_NUMBER<\/a> Left shift of a negative signed number is undefined behaviorThis warning indicates we are left shifting a negative signed integral value, which is a bad idea and triggers implementation defined behavior.\n<pre class=\"prettyprint\">\n\/\/ Example source:\nvoid leftshift(int shiftCount) {\n    const auto result = -1 &lt;&lt; shiftCount; \/\/ C26453 reported here\n        \/\/ code\n}\n<\/pre>\n<pre class=\"prettyprint\">\n\/\/ Corrected source:\nvoid leftshift(int shiftCount) {\n    const auto result = 4294967295 &lt;&lt; shiftCount; \/\/ OK\n         \/\/ code\n}\n<\/pre>\n<p>In the corrected source, a positive integral value was used for the shift operation.<\/li>\n<li><a href=\"https:\/\/docs.microsoft.com\/visualstudio\/code-quality\/c26454\">C26454 RESULT_OF_ARITHMETIC_OPERATION_NEGATIVE_UNSIGNED<\/a> [operator] operation wraps past 0 and produces a large unsigned number at compile timeThis 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.\n<pre class=\"prettyprint\">\n\/\/ Example source:\nunsigned int negativeunsigned() {\n    const unsigned int x = 1u - 2u; \/\/ C26454 reported here\n    return x;\n}\n<\/pre>\n<pre class=\"prettyprint\">\n\/\/ Corrected source:\nunsigned int negativeunsigned() {\n    const unsigned int x = 4294967295; \/\/ OK\n    return x;\n}\n<\/pre>\n<p>In the corrected source, a positive value was assigned to the unsigned result.<\/li>\n<\/ul>\n<h3>Results<\/h3>\n<p>We ran these checks in a security-sensitive codebase over the holidays and found interesting bug patterns. I am re-sharing the example patterns that looked suspicious at the beginning of this blog post along with the code analysis warnings they now trigger.<\/p>\n<p><code>Warning\u00a0C26451\u00a0Arithmetic 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<\/code><\/p>\n<pre class=\"prettyprint\">\nuint32 ConvertOffset(uint32 ptr, uint32 fromSize, uint32 toSize)\n{\n    if (fromSize == toSize)\n    {\n        return ptr;\n    }\n    uint64 tmp = ptr * fromSize; \/\/ C26451\n    \/\/ More code\n}\n<\/pre>\n<p><code>Warning C26454\u00a0Arithmetic overflow: '-' operation produces a negative unsigned result at compile time, resulting in an overflow<\/code><\/p>\n<pre class=\"prettyprint\">\ntemplate\nclass BucketEntryIterator sealed : public IteratorBase&amp;lt;TDictionary, BucketEntryIterator&amp;gt;\n{\nprivate:\n    uint bucketIndex;\n    \/\/ Rest of the data members\n\npublic:\n    BucketEntryIterator(TDictionary &amp;amp;dictionary)\n    : Base(dictionary, -1),\n      bucketIndex(0u \u2013 1) \/\/ C26454\n      \n      \/\/ Initialize other data members of the class\n};\n<\/pre>\n<h3>Feedback<\/h3>\n<p>We\u2019d love for you to try these checks firsthand. <a href=\"https:\/\/www.visualstudio.com\/vs\/preview\/\">Download Version 15.6 Preview 6<\/a> and let us know if you find any\u00a0interesting bug patterns in your codebase.<\/p>\n<p>As always, if you have any feedback or suggestions for us, let us know. We can be reached via the comments below, via email (<a href=\"mailto:visualcpp@microsoft.com\">visualcpp@microsoft.com<\/a>) and you can provide feedback via <a href=\"https:\/\/docs.microsoft.com\/en-us\/visualstudio\/ide\/how-to-report-a-problem-with-visual-studio-2017\">Help &gt; Report A Problem in the product<\/a>, or via <a href=\"https:\/\/developercommunity.visualstudio.com\/topics\/C%2B%2B.html\">Developer Community<\/a>. You can also find us on Twitter (<a href=\"https:\/\/twitter.com\/visualc\">@VisualC<\/a>) and Facebook (<a href=\"https:\/\/www.facebook.com\/msftvisualcpp\">msftvisualcpp<\/a>).<\/p>\n","protected":false},"excerpt":{"rendered":"<p>\u70b9\u8fd9\u91cc\u770b\u4e2d\u6587\u7248 We&#8217;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\u2019ll want to enable them in your code. If you\u2019re just getting started with C++ Code Analysis in Visual [&hellip;]<\/p>\n","protected":false},"author":324,"featured_media":35994,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"_acf_changed":false,"footnotes":""},"categories":[239,277],"tags":[245,163],"class_list":["post-18306","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-diagnostics","category-writing-code","tag-cppcorecheck","tag-static-analysis"],"acf":[],"blog_post_summary":"<p>\u70b9\u8fd9\u91cc\u770b\u4e2d\u6587\u7248 We&#8217;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\u2019ll want to enable them in your code. If you\u2019re just getting started with C++ Code Analysis in Visual [&hellip;]<\/p>\n","_links":{"self":[{"href":"https:\/\/devblogs.microsoft.com\/cppblog\/wp-json\/wp\/v2\/posts\/18306","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/devblogs.microsoft.com\/cppblog\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/devblogs.microsoft.com\/cppblog\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/cppblog\/wp-json\/wp\/v2\/users\/324"}],"replies":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/cppblog\/wp-json\/wp\/v2\/comments?post=18306"}],"version-history":[{"count":0,"href":"https:\/\/devblogs.microsoft.com\/cppblog\/wp-json\/wp\/v2\/posts\/18306\/revisions"}],"wp:featuredmedia":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/cppblog\/wp-json\/wp\/v2\/media\/35994"}],"wp:attachment":[{"href":"https:\/\/devblogs.microsoft.com\/cppblog\/wp-json\/wp\/v2\/media?parent=18306"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/cppblog\/wp-json\/wp\/v2\/categories?post=18306"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/cppblog\/wp-json\/wp\/v2\/tags?post=18306"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}