Leo Tolstoy and Static Code Analysis

Apache HTTP Server vs PVS-Studio

We've checked Apache HTTP Server with PVS-Studio and as we expected, we found errors. The errors are few, which we also expected.

Other developers also come across this situation while testing PVS-Studio on their projects. Unfortunately, the first conclusion you would like to draw on seeing just a few errors is that a tool like this is useless to you. I have just invented a good analogy to show why this is not so.

War and Peace

Imagine the following situation. We have a work by Leo Tolstoy, for instance, "War and Peace". We want to test the mechanism of spelling check integrated into Microsoft Word on this book. We launch analysis and find just a few real errors and a lot of false reports. Besides, Leo Tolstoy was apt to write long sentences. Microsoft Word, being unable to understand the text, will underline a lot of fragments with green supposing that there are punctuation mistakes. We are looking at this all and turning our nose up at it. We don't like that Word considers many names incorrect. We don't like the punctuation analysis. We conclude that spelling check in Word is an absolutely useless thing.

Let's find out what is wrong about this way of studying the tool.

We forget that people have spent a lot of time and effort working on errors in the book. Leo Tolstoy rewrote his drafts and eliminated mistakes. The editor worked turning the text into the book. Something was fixed in the next editions.

Of course, the program will not find all the mistakes in the book. People can do it better. But the program does it much faster and therefore reduces the waste of human effort. The true advantage of this tool lies in its regular use: you have just written a paragraph, noticed an underlined mistake and fixed it instantly. Now, while rereading the text, a person can concentrate on the contents and subtle mistakes instead of fixing trivial misprints.

Remarks on the issue of false reports sound unconvincing too. You only need to add an unknown word to the dictionary once and it will no longer bother the text's author. For the rest of the obviously false warnings you can click the right mouse button and select "ignore".

I think it's unreasonable to argue on the usefulness of text checking mechanisms embedded in such programs as Word, Thunderbird and TortoiseSVN. We use them and don't worry that they find few mistakes in the book "War and Peace ".

There is a similar situation with static analysis of software source code. It's meaningless to check a project like Apache HTTP Server and try to estimate a probable profit to be drawn from the analysis. The code of Apache HTTP Server is old from the viewpoint of programming. As a project, Apache HTTP Server has existed for more than 15 years. The code's quality is confirmed by a huge amount of projects based on it. Of course, a lot of errors were fixed due to the wide use of the code by plenty of programmers and a long development time.

It's logical to suppose that many of the errors found earlier could have been found much easier if static analysis had been used. The analogy with Word is complete in this case. Regular search of errors is much more useful than a one-time search.

As in the Word editor, false reports in PVS-Studio can be easily eliminated: you just have to click on a false message and hide it (a special comment will be automatically added into the corresponding code fragment).

Let's make the final comparison by the parameter of error detection speed. The Word editor can underline mistakes at once. The PVS-Studio analyzer does it after compiling the files. This can't be done earlier because the syntax is too complicated to analyze incomplete code. It's enough, however. We may consider PVS-Studio as a tool to get more compiler-generated warnings. By the way, PVS-Studio can do it in various Visual Studio versions: 2005, 2008, 2010. So you are welcome to try the incremental analysis.

All in all, I find the analogy with Word absolute. If somebody disagrees, we can continue the discussion in comments or via e-mail (karpov [@] viva64.com).

I have formulated the main idea I wanted to share with you. However, I would disappoint the readers if I didn't write some examples of errors found in Apache HTTP Server. So, let's speak a bit on the errors detected.

An example of unnecessary sizeof().

PSECURITY_ATTRIBUTES GetNullACL(void)
{
  PSECURITY_ATTRIBUTES sa;

  sa  = (PSECURITY_ATTRIBUTES) LocalAlloc(LPTR,
           sizeof(SECURITY_ATTRIBUTES));
  sa->nLength = sizeof(sizeof(SECURITY_ATTRIBUTES));
  ...
}

PVS-Studio's diagnostic message is the following: V568 It's odd that the argument of sizeof() operator is the 'sizeof (SECURITY_ATTRIBUTES)' expression. libhttpd util_win32.c 115

The size of the SECURITY_ATTRIBUTES structure is set incorrectly. As a result, correct handling of this structure is impossible.

However, I suspect that most errors I will cite here are located in code that gets control very rarely. That's why you shouldn't worry too much. The next sample is similar. The error is in the function responsible for error handling.

apr_size_t ap_regerror(int errcode, const ap_regex_t *preg,
  char *errbuf, apr_size_t errbuf_size)
{
  ...
  apr_snprintf(errbuf, sizeof errbuf,
               "%s%s%-6d", message, addmessage, (int)preg->re_erroffset);
  ...
}

PVS-Studio's diagnostic message is the following: V579 The apr_snprintf function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the second argument. libhttpd util_pcre.c 85

It means that it is the pointer size that is passed into a function instead of the buffer size. As a result, the error message is formulated incorrectly. Actually the message's length will take 3 or 7 characters.

There are also classic Copy-Paste errors.

static int magic_rsl_to_request(request_rec *r)
{
  ...
  if (state == rsl_subtype || state == rsl_encoding ||
      state == rsl_encoding) {
  ...
}

PVS-Studio's diagnostic message is the following: V501 There are identical sub-expressions 'state == rsl_encoding' to the left and to the right of the '||' operator. mod_mime_magic mod_mime_magic.c 787

The comparison really must look this way:

if (state == rsl_subtype || state == rsl_separator ||
    state == rsl_encoding) {

There are errors that will occur only in Windows operating system.

typedef UINT_PTR SOCKET;
static unsigned int __stdcall win9x_accept(void * dummy)
{
  SOCKET csd;
  ...
  do {
      clen = sizeof(sa_client);
      csd = accept(nsd, (struct sockaddr *) &sa_client, &clen);
  } while (csd < 0 && APR_STATUS_IS_EINTR(apr_get_netos_error()));
  ...
}

PVS-Studio's diagnostic message is the following: V547 Expression 'csd < 0' is always false. Unsigned type value is never < 0. libhttpd child.c 404

The accept function in the Visual Studio header files returns a value that has the unsigned SOCKET type. That's why the check 'csd < 0' is invalid since its result is always false. The returned values must be explicitly compared to different constants, for instance, SOCKET_ERROR.

There is a sort of errors that can be referred to potential vulnerabilities. I don't know if it is possible, at least in theory, to use them for attack. Still, I want to give a couple of such samples here.

typedef  size_t apr_size_t;
APU_DECLARE(apr_status_t) apr_memcache_getp(...)
{
  ...
  apr_size_t len = 0;
  ...
  len = atoi(length);
  ...
  if (len < 0) {
    ...
  }
  else {
    ...  
  }
}

PVS-Studio's diagnostic message is the following: V547 Expression 'len < 0' is always false. Unsigned type value is never < 0. aprutil apr_memcache.c 814

The "len < 0" condition in this code is always false since the 'len' variable gas an unsigned type. Correspondingly, if we send a string with a negative number to input, a failure is likely to occur.

Apache also has much code where various buffers with data are being cleared. Most likely, it is determined by the safety purposes. But clearing itself often fails because of the following error:

#define MEMSET_BZERO(p,l)       memset((p), 0, (l))

void apr__SHA256_Final(sha2_byte digest[], SHA256_CTX* context) {
  ...
  MEMSET_BZERO(context, sizeof(context));
  ...
}

PVS-Studio's diagnostic message is the following: V512 A call of the 'memset' function will lead to underflow of the buffer '(context)'. apr sha2.c 560

Only the first bytes in the buffer are being cleared because the sizeof() operator returns the pointer size instead of the size of the data buffer. I've found at least 6 such errors.

The remaining errors are boring, so I won't write about them. Thank you for your attention. Come and try PVS-Studio. We still give keys to developers of free open-source projects and those who feel a strong urge to try the full analyzer. Write to us.



About the Author

Andrey Karpov

Andrey Karpov is technical manager of the OOO "Program Verification Systems" (Co Ltd) company developing the PVS-Studio tool which is a package of static code analyzers integrating into the Visual Studio development environment.

PVS-Studio is a static analyzer that detects errors in source code of C/C++ applications. There are 3 sets of rules included into PVS-Studio:

  1. Diagnosis of 64-bit errors (Viva64)
  2. Diagnosis of parallel errors (VivaMP)
  3. General-purpose diagnosis

Andrey Karpov is also the author of many articles on the topic of 64-bit and parallel software development. To learn more about the PVS-Studio tool and sources concerning 64-bit and parallel software development, please visit the www.viva64.com site.

Best Articles:

My page on LinkedIn site: http://www.linkedin.com/pub/4/585/6a3

E-mail: karpov@viva64(dot)com

Comments

  • pFAba yRo SyYM

    Posted by SlfHELmXbZ on 06/20/2013 04:43pm

    buy vicodin where to buy vicodin in los angeles - vicodin withdrawal addiction

    Reply
  • effekten er en smuk hovedtelefoner

    Posted by wanzixiao on 06/04/2013 12:14pm

    [url=http://www.nyebeatsbydrdre.350.com/]Nye beats by dr dre[/url] Fordelene og ulemperne ved de forskellige typer af hovedtelefonerHeadset hovedtelefoner, takket være en større enhed, lydkvaliteten er mere bedre, men på grund af nogle store volumen, ikke er nem at bære. Ørepropper lille enhed design, kompakt og let at bære, velegnet til udendørs brug. Ørekrog øretelefon typen headset med ørepropper i mellem et smukt udseende, komfortable at bære, lyde bedre egenskaber end ørepropper ulempe er følsom over for støj udefra, som er egnet til at bære i foråret, og denne sæson . Forskellige typer hovedtelefoner er anderledes, for at se om du ønsker at bruge under hvilke omstændigheder. [url=http://www.beatsbydrdredanmark.weebly.com/]Beats by dre danmark[/url] Based hovedtelefoner tilbyde lydkvalitet, der er lige så forbløffende. Ideel til nutidens digitale musik, disse hovedtelefoner giver dyb bas, en fed mellemtone, og klar, uforvrænget diskant, så du kan høre hver detail.The on-ear, lukket-back design giver langvarig komfort og en høj grad af naturlig støj isolation . Den enkeltsidede kabel er designet til at give dig masser af slæk mens modstå tangles. Integreret i kablet er Monsters ControlTalk modul, som giver dig en kontrolknap og high-grade mikrofon, ideel til styring musikafspilning, telefonopkald og tage stemmenotater på kompatible iPods, iPhones, og Blackberrys. Den guldbelagt 3,5 mm stik giver bred kompatibilitet med MP3-afspillere, cd-afspillere, computere og meget mere. [url=http://www.beatsbydrdredanmark.webgarden.com/]Beats by dre billig pris[/url] Beats “Supreme Sound” Headsets forskydninger, der er valgt i form af vej erklæring – selv om det faktum, at faktisk syntes at tale om! “Jeg er en god overdreven person i hans teenageår” The hovedtelefon anker var oprindeligt strålende sammen med betydelig med hensyn til neon . Men i tidligere tider flere aldre, har kunderne allerede blevet anvendt som vil high-end konkurrerende virksomheder anvender personlig bank kan synes, sammen med Beats indgået et system var oprindeligt det meste af tydeligt. Artiklen udformet en slags in-house industri team – tidligere var næsten hele pakken jobbet oprindeligt dyrket ud – at drage fordel af luftstrøm sammen med forbedre enhver mp3. Den populære sti i disse dage har en god imprintede Great Tone.

    Reply
Leave a Comment
  • Your email address will not be published. All fields are required.

Top White Papers and Webcasts

  • A modern mobile IT strategy is no longer an option, it is an absolute business necessity. Today's most productive employees are not tied to a desk, an office, or a location. They are mobile. And your company's IT strategy has to be ready to support them with easy, reliable, 24/7 access to the business information they need, from anywhere in the world, across a broad range of communication devices. Here's how some of the nation's most progressive corporations are meeting the many needs of their mobile workers …

  • As mobile devices have pushed their way into the enterprise, they have brought cloud apps along with them. This app explosion means account passwords are multiplying, which exposes corporate data and leads to help desk calls from frustrated users. This paper will discover how IT can improve user productivity, gain visibility and control over SaaS and mobile apps, and stop password sprawl. Download this white paper to learn: How you can leverage your existing AD to manage app access. Key capabilities to …

Most Popular Programming Stories

More for Developers

Latest Developer Headlines

RSS Feeds