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.

>
>
>
Analyzing the Network Security Services…

Analyzing the Network Security Services Library

Oct 08 2014

Network Security Services (NSS) is a set of libraries designed to support cross-platform development of security-enabled client and server applications. It implements cryptographic functions in the Firefox and Chrome browsers, and after a recently found certificate signature verification vulnerability, I decided to take a look at this project too.

0286_NSS/image1.png

More about the vulnerability.

We obtained the source code through the following commands:

  • hg clone https://hg.mozilla.org/projects/nspr
  • hg clone https://hg.mozilla.org/projects/nss

Since the library is built from the Windows console, I had to use a special utility PVS-Studio Standalone to analyze it. This tool is described in the article PVS-Studio Now Supports Any Build System under Windows and Any Compiler. Easy and Right Out of the Box.

Analysis results

V547 Expression 'dtype != 2 || dtype != 3' is always true. Probably the '&&' operator should be used here. crlgen.c 1172

static SECStatus
crlgen_setNextDataFn_field(...., unsigned short dtype)
{
  ....
  if (dtype != CRLGEN_TYPE_DIGIT ||                    // <=
      dtype != CRLGEN_TYPE_DIGIT_RANGE) {              // <=
        crlgen_PrintError(crlGenData->parsedLineNum,
          "range value should have "
          "numeric or numeric range values.\n");
    return SECFailure;
  }
  ....
}

The truth table of logical disjunction suggests that if at least one operand is one (like in this case), the condition will always be true.

V567 Undefined behavior. The 'j' variable is modified while being used twice between sequence points. pk11slot.c 1934

PK11SlotList* PK11_GetAllTokens(....)
{
  ....
  #if defined( XP_WIN32 ) 
    waste[ j & 0xf] = j++; 
  #endif
  ....
}

The 'j' variable is used twice in one sequence point. Because of that, you can't predict the result of this expression. To learn more, see the description of the V567 diagnostic.

V575 The null pointer is passed into 'fclose' function. Inspect the first argument. certcgi.c 608

static int get_serial_number(Pair  *data)
{
  FILE *serialFile;
  ....
  serialFile = fopen(filename, "r");
  if (serialFile != NULL) {
  ....
  } else {
    fclose(serialFile);                  // <=
    ....
  }
  ....
}

In this case, the file shouldn't be closed if the pointer to the file is null. Otherwise, it will cause some troubles. What exactly will happen depends on the handler used to handle such issues (see _set_invalid_parameter_handler() and so on).

V576 Incorrect format. A different number of actual arguments is expected while calling 'fprintf' function. Expected: 3. Present: 7. pp.c 34

static void Usage(char *progName)
{
  ....
  fprintf(stderr, "%-14s (Use either the long type name or "
    "the shortcut.)\n", "", SEC_CT_CERTIFICATE_ID,
    SEC_CT_PKCS7, SEC_CT_CRL, SEC_CT_NAME);
  ....
}

The number of format specifiers doesn't correspond to the number of arguments passed to the fprintf() function.

V595 The 'buf' pointer was utilized before it was verified against nullptr. Check lines: 1709, 1710. prtime.c 1709

PR_IMPLEMENT(PRUint32) PR_FormatTime(....)
{
  ....
  rv = strftime(buf, buflen, fmt, ap);
  if (!rv && buf && buflen > 0) {
    buf[0] = '\0';
  }
  return rv;
}

The 'buf' pointer is still checked for being null. It means that an error may occur in the previous line when passing a null pointer to the strftime() function.

V597 The compiler could delete the 'memset' function call, which is used to flush 'hashed_secret' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. alghmac.c 87

#define PORT_Memset memset

SECStatus
HMAC_Init( HMACContext * cx, const SECHashObject *hash_obj,
           const unsigned char *secret,
           unsigned int secret_len, PRBool isFIPS)
{
  ....
  PORT_Memset(hashed_secret, 0, sizeof hashed_secret);   // <=
  if (cx->hash != NULL)
    cx->hashobj->destroy(cx->hash, PR_TRUE);
  return SECFailure;
}

This is the most dangerous fragment in code responsible for processing confidential information. Since the 'hashed_secret' array is not used anymore after calling the 'memset' function, the compiler is allowed to delete the function call for the sake of optimization, and so the array won't be cleared as intended.

Those were probably the most dangerous errors among all found.

Programmers often don't quite understand the V597 warning. So here you are some additional materials to figure out what this issue is all about:

Here is the list of all such fragments:

  • V597 The compiler could delete the 'memset' function call, which is used to flush 'ctx' object. The RtlSecureZeroMemory() function should be used to erase the private data. sha512.c 503
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'ctx' object. The RtlSecureZeroMemory() function should be used to erase the private data. sha512.c 605
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'ctx' object. The RtlSecureZeroMemory() function should be used to erase the private data. sha512.c 1307
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'ctx' object. The RtlSecureZeroMemory() function should be used to erase the private data. sha512.c 1423
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'cx' object. The RtlSecureZeroMemory() function should be used to erase the private data. md5.c 209
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'ctx' object. The RtlSecureZeroMemory() function should be used to erase the private data. sha_fast.c 416
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'lastBlock' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. cts.c 141
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'lastBlock' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. cts.c 299
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'data' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. drbg.c 300
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'inputhash' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. drbg.c 450
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'localDigestData' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. dsa.c 417
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'U' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. pqg.c 422
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'sha1' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. pqg.c 423
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'sha2' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. pqg.c 424
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'U' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. pqg.c 471
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'data' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. pqg.c 1208
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'state' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. tlsprfalg.c 86
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'outbuf' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. tlsprfalg.c 87
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'newdeskey' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. pkcs11c.c 943
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'randomData' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. pk11merge.c 298
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'keyData' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. sslcon.c 2151
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'randbuf' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. keystuff.c 113

V610 Undefined behavior. Check the shift operator '<<. The left operand '-1L' is negative. inflate.c 1475

long ZEXPORT inflateMark(strm)
z_streamp strm;
{
  struct inflate_state FAR *state;

  if (strm == Z_NULL || strm->state == Z_NULL)
    return -1L << 16;
  state = (struct inflate_state FAR *)strm->state;
  ....
}

According to the C++11 language standard, shifting a negative number causes undefined behavior.

Another similar fragment:

  • V610 Undefined behavior. Check the shift operator '<<=. The left operand is negative ('cipher' = [-1..15]). strsclnt.c 1115

V555 The expression 'emLen - reservedLen - inputLen > 0' will work as 'emLen - reservedLen != inputLen'. rsapkcs.c 708

#define PORT_Memset memset

static SECStatus
eme_oaep_encode(unsigned char * em,
                unsigned int emLen,
                const unsigned char * input,
                unsigned int inputLen,
                HASH_HashType hashAlg,
                HASH_HashType maskHashAlg,
                const unsigned char * label,
                unsigned int labelLen,
                const unsigned char * seed,
                unsigned int seedLen)
{
  ....
  /* Step 2.b - Generate PS */
    if (emLen - reservedLen - inputLen > 0) {
        PORT_Memset(em + 1 + (hash->length * 2), 0x00,
                    emLen - reservedLen - inputLen);
    }
  ....
}

Besides a correct number and zero, difference of unsigned numbers may result in an extremely large value resulting from casting a negative number to unsigned. In this fragment, an incorrect giant value will meet the condition and the 'memset' function will try to clear a huge amount of memory.

However, such an overflow might never occur at all - one can't say for sure what the limits of the range of values the variables in this expression can possibly take are. But the check is too unsafe anyway.

V677 Custom declaration of a standard 'BYTE' type. The system header file should be used: #include <WinDef.h>. des.h 15

typedef unsigned char BYTE;

And the last thing, a small comment concerning the issue with declaring types which are already declared in system files.

Check the following fragments:

  • V677 Custom declaration of a standard 'WORD' type. The system header file should be used: #include <WinDef.h>. arcfour.c 36
  • V677 Custom declaration of a standard 'off_t' type. The system header file should be used: #include <WCHAR.H or SYS\TYPES.H>. winfile.h 34

That's not an error of course. But still, why do that?

Conclusion

Security of private data has been treated with especially high attention recently. So let's not forget that software security means as well as intrusion means are developed by humans, and humans tend to make mistakes.

Using static analysis regularly will help you save plenty of time to solve more serious tasks.

Popular related articles
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…
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…
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…
Technologies used in the PVS-Studio code analyzer for finding bugs and potential vulnerabilities

Date: Nov 21 2018

Author: Andrey Karpov

A brief description of technologies used in the PVS-Studio tool, which let us effectively detect a large number of error patterns and potential vulnerabilities. The article describes the implementati…
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…
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…
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…
The Ultimate Question of Programming, Refactoring, and Everything

Date: Apr 14 2016

Author: Andrey Karpov

Yes, you've guessed correctly - the answer is "42". In this article you will find 42 recommendations about coding in C++ that can help a programmer avoid a lot of errors, save time and effort. The au…
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…

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