To get a trial key
fill out the form below
Team License (standard version)
Enterprise License (extended version)
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
Request our prices
New License
License Renewal
--Select currency--
USD
EUR
GBP
RUB
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
To get the licence for your open-source project, please fill out this form
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
To get the licence for your open-source project, please fill out this form
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
I am interested to try it on the platforms:
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
Message submitted.

Your message has been sent. We will email you at


If you haven't received our response, please do the following:
check your Spam/Junk folder and click the "Not Spam" button for our message.
This way, you won't miss messages from our team in the future.

>
>
>
How developers were checking projects f…

How developers were checking projects for bugs using PVS-Studio

Dec 04 2017

Pinguem.ru and the PVS-Studio team recently held a contest where programmers were to use PVS-Studio static analyzer for one month to find and fix bugs in the source code of open-source projects. Their efforts have helped make a great many applications a tiny bit safer and more reliable. In this article, we will discuss a few of the most interesting bugs found with the help of PVS-Studio.

0544_Results_of_Pinguem_contest/image1.png

So, how did it go?

The contest was held for the Russian-speaking community from October 23 to November 27, 2017, and was split into two stages. At the first stage, the contestants were to submit as many pull requests to the project authors as possible. The second stage was a bit more challenging: they were asked to find a bug and describe the sequence of steps to reproduce it. Nikolay Shalakin was the one who scored the most points and won the contest. Congratulations, Nikolay!

During the contest, the participants submitted a lot of really useful pull requests, all of which are listed here. As for this article, we invite you to take a look at the most interesting bugs found by the contestants at the second stage.

QtCreator

How many of you use QtCreator when coding in Python? Like many other IDEs, it highlights some of the built-in functions and objects. Let's run QtCreator 4.4.1 and write a few keywords:

0544_Results_of_Pinguem_contest/image2.gif

What's that? Why doesn't it highlight built-in functions oct and chr? Let's look into their code:

// List of python built-in functions and objects
static const QSet<QString> builtins = {
"range", "xrange", "int", "float", "long", "hex", "oct" "chr", "ord",
"len", "abs", "None", "True", "False"
};

The function declarations are fine; what's wrong then? PVS-Studio clarifies the issue:

V653 A suspicious string consisting of two parts is used for initialization. It is possible that a comma is missing. Consider inspecting this literal: "oct" "chr". pythonscanner.cpp 205

Indeed, the programmer forgot to write a comma between literals "oct" and "chr", so the two have merged into one, "octchr", and it is this literal that QtCreator highlights:

0544_Results_of_Pinguem_contest/image3.gif

The bug-fix pull request can be found here.

ConEmu

Suppose you are working on a ConEmu project and want to check on some of the settings in the debug version (click on the animation to enlarge):

0544_Results_of_Pinguem_contest/image4.gif

Let's look into the code to find out why we get the "ListBox was not processed" message:

INT_PTR CSetPgViews::OnComboBox(HWND hDlg, WORD nCtrlId, WORD code)
{
  switch (code)
  {
  ....
  case CBN_SELCHANGE:
    {
      ....
      UINT val;
      INT_PTR nSel = SendDlgItemMessage(hDlg, 
                                        nCtrlId, 
                                        CB_GETCURSEL,
                                        0,
                                        0);
      switch (nCtrlId)
      {
        ....
        case tThumbMaxZoom:
          gpSet->ThSet.nMaxZoom = max(100,((nSel+1)*100));
        default:
          _ASSERTE(FALSE && "ListBox was not processed");
      }
    }
  }
}

Because of the missing break statement, the control will pass to the default branch after executing the expressions in the tThumbMaxZoom branch. That's just what PVS-Studio warns us about:

V796 It is possible that 'break' statement is missing in switch statement. setpgviews.cpp 183

The bug-fix pull request can be found here.

UniversalPauseButton

This project is quite an interesting one and is especially useful for gamers. When you click on the Pause key, the program pauses the foreground window's operation:

You can reassign the pause/resume function to another key by tweaking the settings.txt file:

0544_Results_of_Pinguem_contest/image6.png

If you enter a key code whose length is no less than 20 characters and no greater than 30 characters, this will result in a stack buffer overflow (click on the animation to enlarge):

0544_Results_of_Pinguem_contest/image7.gif

Let's find out why it happens. We are interested in function LoadPauseKeyFromSettingsFile:

int LoadPauseKeyFromSettingsFile(_In_ wchar_t* Filename)
{
  HANDLE FileHandle = CreateFile(Filename, 
                                 GENERIC_READ,
                                 FILE_SHARE_READ,
                                 NULL,
                                 OPEN_EXISTING,
                                 FILE_ATTRIBUTE_NORMAL,
                                 NULL);

  if (FileHandle == INVALID_HANDLE_VALUE)
  {
    goto Default;
  }
  
  char  KeyLine[32] = { 0 };
  char  Buffer[2]   = { 0 };
  DWORD ByteRead    = 0;

  do
  {
    if (!ReadFile(FileHandle, Buffer, 1, &ByteRead, NULL))
    {
      goto Default;
    }

    if (Buffer[0] == '\r' || Buffer[0] == '\n')
    {
      break;
    }

    size_t Length = strlen(KeyLine);
    if (Length > 30)                                            // <=
    {
      goto Default;
    }

    KeyLine[Length] = Buffer[0];    
    memset(Buffer, 0, sizeof(Buffer));
  } while (ByteRead == 1);

  if (!StringStartsWith_AI(KeyLine, "KEY="))
  {
    goto Default;
  }

  char KeyNumberAsString[16] = { 0 };                           // <=

  for (DWORD Counter = 4; Counter < strlen(KeyLine); Counter++) // <=
  {
    KeyNumberAsString[Counter - 4] = KeyLine[Counter];
  }
  ....

  Default:
  if (FileHandle != INVALID_HANDLE_VALUE && FileHandle != NULL)
  {
    CloseHandle(FileHandle);    
  }
  return(0x13);
}

In the loop above, the first string is read byte by byte. If its length is greater than 30 characters, control passes to the Default label, releasing the resource and returning character code 0x13. If the string has been read successfully and the first string starts with "KEY=", the substring following the "=" character is copied into 16-byte buffer KeyNumberAsString. Entering a key code from 20 to 30 characters long would result in a buffer overflow. That's just what PVS-Studio warns us about:

V557 Array overrun is possible. The value of 'Counter - 4' index could reach 26. main.cpp 146

The bug-fix pull request can be found here.

Explorer++

The bug found in this project has to do with bookmark sort (click on the animation to enlarge):

0544_Results_of_Pinguem_contest/image9.gif

Let's examine the code performing the sort:

int CALLBACK SortByName(const NBookmarkHelper::variantBookmark_t
                          BookmarkItem1,
                        const NBookmarkHelper::variantBookmark_t
                          BookmarkItem2)
{
  if (   BookmarkItem1.type() == typeid(CBookmarkFolder)
      && BookmarkItem2.type() == typeid(CBookmarkFolder))
  {
    const CBookmarkFolder &BookmarkFolder1 =
      boost::get<CBookmarkFolder>(BookmarkItem1);
    const CBookmarkFolder &BookmarkFolder2 =
      boost::get<CBookmarkFolder>(BookmarkItem2);

    return BookmarkFolder1.GetName()
           .compare(BookmarkFolder2.GetName());
  }
  else
  {
    const CBookmark &Bookmark1 = 
      boost::get<CBookmark>(BookmarkItem1);
    const CBookmark &Bookmark2 =
      boost::get<CBookmark>(BookmarkItem1);

    return Bookmark1.GetName().compare(Bookmark2.GetName());
  }
}

The programmer made a mistake in the else branch and used BookmarkItem1 twice instead of using BookmarkItem2 in the second case. That's just what PVS-Studio warns us about:

  • V537 Consider reviewing the correctness of 'BookmarkItem1' item's usage. bookmarkhelper.cpp 535
  • another 5 warnings.

The bug-fix pull request can be found here.

Conclusion

The PVS-Studio team is very grateful to all the participants. You did a tremendous job eliminating bugs in open-source projects, making them better, safer, and more reliable. Perhaps someday we'll have a similar contest for the English-speaking community as well.

All the rest are welcome to download and try PVS-Studio analyzer. It is very easy to use and could help you a lot.

Popular related articles
Characteristics of PVS-Studio Analyzer by the Example of EFL Core Libraries, 10-15% of False Positives

Date: Jul 31 2017

Author: Andrey Karpov

After I wrote quite a big article about the analysis of the Tizen OS code, I received a large number of questions concerning the percentage of false positives and the density of errors (how many erro…
Appreciate Static Code Analysis!

Date: Oct 16 2017

Author: Andrey Karpov

I am really astonished by the capabilities of static code analysis even though I am one of the developers of PVS-Studio analyzer myself. The tool surprised me the other day as it turned out to be sma…
The Evil within the Comparison Functions

Date: May 19 2017

Author: Andrey Karpov

Perhaps, readers remember my article titled "Last line effect". It describes a pattern I've once noticed: in most cases programmers make an error in the last line of similar text blocks. Now I want t…
The way static analyzers fight against false positives, and why they do it

Date: Mar 20 2017

Author: Andrey Karpov

In my previous article I wrote that I don't like the approach of evaluating the efficiency of static analyzers with the help of synthetic tests. In that article, I give the example of a code fragment…
Static analysis as part of the development process in Unreal Engine

Date: Jun 27 2017

Author: Andrey Karpov

Unreal Engine continues to develop as new code is added and previously written code is changed. What is the inevitable consequence of ongoing development in a project? The emergence of new bugs in th…
PVS-Studio ROI

Date: Jan 30 2019

Author: Andrey Karpov

Occasionally, we're asked a question, what monetary value the company will receive from using PVS-Studio. We decided to draw up a response in the form of an article and provide tables, which will sho…
Free PVS-Studio for those who develops open source projects

Date: Dec 22 2018

Author: Andrey Karpov

On the New 2019 year's eve, a PVS-Studio team decided to make a nice gift for all contributors of open-source projects hosted on GitHub, GitLab or Bitbucket. They are given free usage of PVS-Studio s…
PVS-Studio for Java

Date: Jan 17 2019

Author: Andrey Karpov

In the seventh version of the PVS-Studio static analyzer, we added support of the Java language. It's time for a brief story of how we've started making support of the Java language, how far we've co…
How PVS-Studio Proved to Be More Attentive Than Three and a Half Programmers

Date: Oct 22 2018

Author: Andrey Karpov

Just like other static analyzers, PVS-Studio often produces false positives. What you are about to read is a short story where I'll tell you how PVS-Studio proved, just one more time, to be more atte…
The Last Line Effect

Date: May 31 2014

Author: Andrey Karpov

I have studied many errors caused by the use of the Copy-Paste method, and can assure you that programmers most often tend to make mistakes in the last fragment of a homogeneous code block. I have ne…

Comments (0)

Next comments

This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
This website uses cookies and other technology to provide you a more personalized experience. By continuing the view of our web-pages you accept the terms of using these files. If you don't want your personal data to be processed, please, leave this site.
Learn More →
Accept