Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: added related vul for cve 2009-1888

...

In this noncompliant code example, the set_flag() function is intended to set the variable sign to 1 if number is positive and -1 if number is negative. However, the programmer neglected to account for number being 0. If number is 0, then sign remains uninitialized. Because sign is uninitialized, and again assuming that the architecture makes use of a program stack, it uses whatever value is at that location in the program stack. This may lead to unexpected or otherwise incorrect program behavior.

Code Block
bgColor#FFCCCC

void set_flag(int number, int *sign_flag) {
  if (sign_flag == NULL) {
    return;
  }
  if (number > 0) {
    *sign_flag = 1;
  }
  else if (number < 0) {
    *sign_flag = -1;
  }
}

void func(int number) {
  int sign;

  set_flag(number, &sign);
  /* use sign */ 
}

Compilers assume that when the address of an uninitialized variable is passed to a function, the variable is initialized within that function. Because compilers frequently fail to diagnose any resulting failure to initialize the variable, the programmer must apply additional scrutiny to ensure the correctness of the code.

...

This defect results from a failure to consider all possible data states (see MSC01-C. Strive for logical completeness). Once the problem is identified, it can be trivially repaired by accounting for the possibility that number can be equal to 0.

Code Block
bgColor#ccccff

void set_flag(int number, int *sign_flag) {
  if (sign_flag == NULL) {
    return;
  }
  if (number >= 0) { /* account for number being 0 */
    *sign_flag = 1;
  } else {
    assert(number < 0);
    *sign_flag = -1;
  }
}

void func(int number) {
  int sign;

  set_flag(number, &sign);
  /* use sign */ 
}

Noncompliant Code Example

Wiki Markup
In this noncompliant code example, the programmer mistakenly fails to set the local variable {{error_log}} to the {{msg}} argument in the {{report_error()}} function \[[mercy 06|AA. C References#mercy 06]\].  Because {{error_log}} has not been initialized, on architectures making use of a program stack, it assumes the value already on the stack at this location, which is a pointer to the stack memory allocated to the {{password}} array.  The {{sprintf()}} call copies data in {{password}} until a null byte is reached. If the length of the string stored in the {{password}} array is greater than the size of the {{buffer}} array, then a buffer overflow occurs.

Code Block
bgColor#FFCCCC

#include <stdio.h>
#include <ctype.h>
#include <string.h>

int do_auth(void) {
  char *username;
  char *password;

  /* Get username and password from user, return -1 if invalid */
}

void report_error(const char *msg) {
  const char *error_log;
  char buffer[24];

  sprintf(buffer, "Error: %s", error_log);
  printf("%s\n", buffer);
}

int main(void) {
  if (do_auth() == -1) {
    report_error("Unable to login");
  }
  return 0;
}

...

In this noncompliant code example, the report_error() function has been modified so that error_log is properly initialized.

Code Block
bgColor#ffcccc

void report_error(const char *msg) {
  const char *error_log = msg;
  char buffer[24];

  sprintf(buffer, "Error: %s", error_log);

  printf("%s\n", buffer);
}

...

In this solution, the magic number is abstracted and the buffer overflow is eliminated.

Code Block
bgColor#ccccff

enum {max_buffer = 24};

void report_error(const char *msg) {
  const char *error_log = msg;
  char buffer[max_buffer];

  snprintf(buffer, sizeof(buffer), "Error: %s", error_log);
  printf("%s\n", buffer);
}

...

A much simpler, less error prone, and better performing compliant solution is shown here:

Code Block
bgColor#ccccff

void report_error(const char *msg) {
  printf("Error: %s\n", msg);
}

...

Klocwork Version 8.0.4.16 can detect violations of this rule with the UNINIT.HEAP.MIGHT, UNINIT.HEAP.MUST, UNINIT.STACK.ARRAY.MIGHT, UNINIT.STACK.ARRAY.MUST, UNINIT.STACK.ARRAY.PARTIAL.MUST, and UNINIT.STACK.MUST checkers.

Related Vulnerabilities

CVE-2009-1888 results from a violation of this recommendation. Some versions of SAMBA (up to 3.3.5) call a function which takes in two potentially unitiliazed variables involving access rights. An attacker can exploit this to bypass the access control list and gain access to protected files [xorl 2009].

Searchfor Search for vulnerabilities resulting from the violation of this rule on the CERT website.

...

Wiki Markup
\[[Flake 06|AA. C References#Flake 06]\]
\[[ISO/IEC 9899:1999|AA. C References#ISO/IEC 9899-1999]\] Section 6.7.8, "Initialization"
\[[ISO/IEC PDTR 24772|AA. C References#ISO/IEC PDTR 24772]\] "LAV Initialization of Variables"
\[[mercy 06|AA. C References#mercy 06]\]
\[[xorl 2009|AA. C References#xorl 2009]\] ["CVE-2009-1888: SAMBA ACLs Uninitialized Memory Read"|http://xorl.wordpress.com/2009/06/26/cve-2009-1888-samba-acls-uninitialized-memory-read/]

...

EXP32-C. Do not access a volatile object through a non-volatile reference      03. Expressions (EXP)      EXP34-C. Ensure a null pointer is not dereferenced