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

  • Instead of only managing projects organizations do need to manage value! "Doing the right things" and "doing things right" are the essential ingredients for successful software and systems delivery. Unfortunately, with distributed delivery spanning multiple disciplines, geographies and time zones, many organizations struggle with teams working in silos, broken lines of communication, lack of collaboration, inadequate traceability, and poor project visibility. This often results in organizations "doing the …

  • With JRebel, developers get to see their code changes immediately, fine-tune their code with incremental changes, debug, explore and deploy their code with ease (both locally and remotely), and ultimately spend more time coding instead of waiting for the dreaded application redeploy to finish. Every time a developer tests a code change it takes minutes to build and deploy the application. JRebel keeps the app server running at all times, so testing is instantaneous and interactive.

Most Popular Programming Stories

More for Developers

Latest Developer Headlines

RSS Feeds