# How developers were checking projects for bugs using PVS\-Studio

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](https://import.viva64.com/docx/blog/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](https://github.com/AskePit) 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](https://github.com/search?o=desc&q=pinguem+pvs&s=comments&type=Issues&utf8=%E2%9C%93)\. 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](https://import.viva64.com/docx/blog/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:

```cpp
// 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](https://pvs-studio.com/en/docs/warnings/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](https://import.viva64.com/docx/blog/0544_Results_of_Pinguem_contest/image3.gif)

The bug\-fix pull request can be found [here](https://github.com/qt-creator/qt-creator/pull/21)\.

### 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\):

![](https://import.viva64.com/docx/blog/0544_Results_of_Pinguem_contest/image4.mp4)

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

```cpp
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](https://pvs-studio.com/en/docs/warnings/v796/) It is possible that 'break' statement is missing in switch statement\. setpgviews\.cpp 183

The bug\-fix pull request can be found [here](https://github.com/Maximus5/ConEmu/pull/1328)\.

### 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:

<https://www.youtube.com/watch?v=bItgybhViQM>

You can reassign the pause/resume function to another key by tweaking the _settings\.txt_ file:

![0544_Results_of_Pinguem_contest/image6.png](https://import.viva64.com/docx/blog/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\):

![](https://import.viva64.com/docx/blog/0544_Results_of_Pinguem_contest/image7.mp4)

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

```cpp
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](https://pvs-studio.com/en/docs/warnings/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](https://github.com/ryanries/UniversalPauseButton/pull/11)\.

### Explorer\+\+

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

![](https://import.viva64.com/docx/blog/0544_Results_of_Pinguem_contest/image9.mp4)

Let's examine the code performing the sort:

```cpp
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](https://pvs-studio.com/en/docs/warnings/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](https://github.com/derceg/explorerplusplus/pull/85)\.

## 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](https://pvs-studio.com/en/pvs-studio/download/) and try PVS\-Studio analyzer\. It is very easy to use and could help you a lot\.