Uploaded image for project: 'SonarCFamily'
  1. SonarCFamily
  2. CPP-2943

Create an MMF for better reporting focused on buffer overflow.

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: C, C++, Objective-C

      Description

      Buffer overflow is around half of SE issues we report.

      These issues can be quite complicated and the way we report them is not always clear/useful.
      There is a lot of information to track the execution flow (not even good enough in the following example) but the useful data-flow is not shown.

      The following code shows an example of a poor buffer overflow issue report. It is so bare that without digging deeper, one could think it is a false positive.
      Inferring the missing information requires in the best case testing things, in the worst cases, it is impossible and the bug report is useless and very noisy.

      The following code shows a simplified version of a real-life example that at first was thought to be a false-positive: 

      typedef unsigned int size_t;size_t wcslen( const wchar_t *str );wchar_t* wmemcpy( wchar_t* dest, const wchar_t* src, size_t count );
      
      bool someFunc();
      
      size_t getSize() {
        return someFunc() ? 0 : 1;
      }
      
      void g() {
        auto l = getSize();
        wchar_t *pwstr = new wchar_t[l];
      
        auto len =  wcslen(pwstr);
        wchar_t *newElement = new wchar_t[len + 1];
      
        wmemcpy(newElement, pwstr, len + 1); // S5782:"wmemcpy" overflows read buffer "pwstr"; passed size "len + 1" exceeds buffer size
      
        delete[] pwstr;
        delete[] newElement;
      }
      

      Note that the reported issue does NOT have any secondary location, so that the only available information to the user is the report shown in the commented-out line only.

      Eventually, the bug comes from the fact that variable l can be assigned value 0 in some code path and then, wmemcpy will access the first element of the zero-sized array pwstr.

      In general,  data flow could be added to help tracking each argument of the faulty function (as an issue can have multiple flows) .
      We are talking of 2 types of data:
      1 - passed size argument (here, 3rd argument to wmemcpy call).
      2 - allocation size of buffers (here, sizes of buffer args 1 and 2)
      In the CFamily plugin, we do not currently report issues with multiple flows and some work would have to done there.

      CSA offers some features to address case 1 (to be detailed) whereas case 2 (the most valuable in the example issue) needs a deeper look/development to be addressed.

      An MMF should be written to detail all this work. 

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              geoffray.adde Geoffray Adde
              Reporter:
              geoffray.adde Geoffray Adde
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: