Pour obtenir une clé
d'essai remplissez le formulaire ci-dessous
Demandez des tariffs
Nouvelle licence
Renouvellement de licence
--Sélectionnez la devise--
USD
EUR
RUB
* En cliquant sur ce bouton, vous acceptez notre politique de confidentialité

Free PVS-Studio license for Microsoft MVP specialists
To get the licence for your open-source project, please fill out this form
** En cliquant sur ce bouton, vous acceptez notre politique de confidentialité.

I am interested to try it on the platforms:
** En cliquant sur ce bouton, vous acceptez notre politique de confidentialité.

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

04 Déc 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
The feedback for our C++ quiz and why it matters

Date: 31 Aoû 2022

Author: Alexey Sarkisov

Earlier we wrote about our quiz for C++ developers. Since the quiz launch, we have been carefully collecting your feedback. Some of the comments were related to the quiz errors, that we obviously sou…
Static code analyzer vs developers. Here we go again.

Date: 11 Jul 2022

Author: Alexey Sarkisov

In mid-May this year we released an updated version of our quiz for C++ developers. It's been 2 months already — keep on reading to learn more about the results!
Solutions to bug-finding challenges offered by the PVS-Studio team at conferences in 2018-2019

Date: 18 Nov 2019

Author: Sergey Khrenov

Hi! Though the 2019 conference season is not over yet, we'd like to talk about the bug-finding challenges we offered to visitors at our booth during the past conferences. Starting with the fall of 20…
Reworking C and C++ front-end — or how we deal with 16-year legacy code in PVS-Studio

Date: 22 Sep 2022

Author: Sergey Larin

In 2022, the PVS-Studio static analyzer for C and C++ turns 16 years old. If the analyzer were a human, it would have already finished high school! This project was started a long time ago, and since…
The feedback for our C++ quiz and why it matters

Date: 31 Aoû 2022

Author: Alexey Sarkisov

Earlier we wrote about our quiz for C++ developers. Since the quiz launch, we have been carefully collecting your feedback. Some of the comments were related to the quiz errors, that we obviously sou…

Comments (0)

Next comments
Unicorn with delicious cookie
Nous utilisons des cookies pour améliorer votre expérience de navigation. En savoir plus
Accepter