Page History

...

In this noncompliant code example, the set_flag() function is intended to set the variable sign to -1 when number is negative or 1. However, the programmer neglected to account for number being 0. If number is 0, then sign remains uninitialized. Because sign is uninitialized, 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.

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;
}

int is_negative(int number) {
  int sign;

  set_flag(number, &sign);

  return sign < 0;
}

...

Note also that unless doing so is prohibitive for performance reasons, an additional defense-in-depth practice worth considering is to initialize local variables immediately after declaration. While compilers and static analysis tools often detect uses of uninitialized variables when they have access to the source code, diagnosing the problem is difficult or impossible when either the initialization or the use takes place in object code the source code of which is inaccessible to the tool.

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;
  }
}

int is_negative(int number) {
  int sign = 0;   /* initialize as a matter of defense-in-depth */

  set_flag(number, &sign);

  return sign < 0;
}

...

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 2006|AA. Bibliography#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, a buffer overflow occurs.

#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.

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.

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:

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

...

In the noncompliant code example below, the function mbrlen() is passed the address of an automatic mbstate_t object that has not been properly initialized, leading to undefined behavior. See undefined behavior 188 in Section J.2 of C99.

void f(const char *mbs) {
  size_t len;
  mbstate_t state;

  len = mbrlen(mbs, strlen(mbs), &state);

  /* ... */
}

...

Before being passed to a multibyte conversion function, an mbstate_t object must be either initialized to the initial conversion state or set to a value that corresponds to the most recent shift state by a prior call to a multibyte conversion function. The compliant solution below sets the mbstate_t object to the initial conversion state by setting it to all zeros.

void f(const char *mbs) {
  size_t len;
  mbstate_t state;

  memset(&state, 0, sizeof state);
  len = mbrlen(mbs, strlen(mbs), &state);

  /* ... */
}

...