EXP33-C. Do not reference uninitialized variables

Local, automatic variables can assume unexpected values if they are used before they are initialized. C99 specifies "If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate" [[ISO/IEC 9899-1999]]. In practice, this value defaults to whichever values are currently stored in stack memory. While uninitialized memory often contains zero, this is not guaranteed. Consequently, uninitialized memory can cause a program to behave in an unpredictable or unplanned manner and may provide an avenue for attack.

In most cases compilers warn about uninitialized variables. These warnings should be resolved as recommended by MSC00-A. Compile cleanly at high warning levels.

Non-Compliant Code Example

In this non-compliant 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, it assumes whatever value is at that location in the program stack. This may lead to unexpected, incorrect program behavior.

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.

Implementation Details

Microsoft Visual Studio 2005, Visual Studio 2008, GCC version 3.4.4, and GCC version 4.1.3 fail to diagnose this error.

Compliant Solution

This defect results from a failure to consider all possible data states (see MSC01-A. 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.

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 */ 
}

Non-Compliant Code Example

In this non-compliant code example, the programmer mistakenly fails to set the local variable error_log to the msg argument in the report_error() function [[mercy 06]]. Because error_log has not been initialized, 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.

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

enum {max_user = 1024};
enum {max_password = 10}; /* sizeof("password\n\0") */

char const *valid_user = "user";
char const *valid_password = "password";

int do_auth(void) {
  char username[max_user];
  char password[max_password];

  puts("Please enter your username: ");
  if (fgets(username, sizeof( username), stdin) == NULL) {
    /* handle error */
  }
  /* trim off ws at end, including newline */
  while (strlen(username) > 0 &&
         isspace( username[ strlen(username) - 1])) {
    username[ strlen(username) - 1] = '\0';
  }

  puts("Please enter your password: ");
  if (fgets(password, sizeof( password), stdin) == NULL) {
     /* handle error */
  }
  /* trim off ws at end, including newline */
  while (strlen(password) > 0 &&
         isspace( password[ strlen(password) - 1])) {
    password[ strlen(password) - 1] = '\0';
  }

  if (!strcmp(username, valid_user) &&
      !strcmp(password, valid_password)) {
    return 0;
  }
  return -1;
}

void report_error(char const *msg) {
  char const *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;
}

Non-Compliant Code Example

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

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

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

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

This solution is still problematic in that a buffer overflow will occur if the NULL-terminated byte string referenced by msg is greater than 17 bytes, including the NULL terminator. The solution also makes use of a "magic number," which should be avoided (see DCL06-A. Use meaningful symbolic constants to represent literal values in program logic).

Compliant Solution

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

enum {max_buffer = 24};

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

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

Compliant Solution

A much simpler, less error prone, and better performing compliant solution is shown below.

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

Risk Assessment

Accessing uninitialized variables generally leads to unexpected program behavior. In some cases these types of flaws may allow the execution of arbitrary code.

This http://www.kb.cert.org/vuls/id/925211 in the OpenSSL package for Debian Linux, and other distributions derived from Debian, is said to reference unitialized memory. One might say that unitialized memory caused the vulnerability, but not directly. The original OpenSSL code utilized initialized memory as an additional source of randomness to an already-randomly-generated key. This generated good keys, but caused the code-auditing tools Valgrind and Purify to issue warnings. Debian tried to fix the warnings with two changes. One actually eliminated eliminated the unitialized memory access, but the other weakened the randomness of the keys.

Automated Detection

The LDRA tool suite V 7.6.0 is able to detect violations of this rule.

Fortify SCA Version 5.0 is able to detect violations of this rule, but will return false positives if the initialization was done in another function.

The tool Compass Rose automatically detects simple violations of this rule, but will not catch more complex violations, such as initialization within functions taking arguments to uninitialized variables, and returns some false positives. Rose does tach 2nd non-compliant code example, and could be extended to catch first, but this would be difficult.

The Coverity Prevent UNINIT checker can find cases of an uninitialized variable being used before it is initialized, although it cannot detect cases of uninitialized members of a struct. Coverity Prevent cannot discover all violations of this rule, so further verification is necessary.

Related Vulnerabilities

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

References

[[Flake 06]]
[[ISO/IEC 9899-1999]] Section 6.7.8, "Initialization"
[[mercy 06]]

EXP32-C. Do not cast away a volatile qualification      03. Expressions (EXP)       EXP34-C. Ensure a NULL pointer is not dereferenced