Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Removed an NCCE/CS that were not required; not satisfactorily reviewed

...

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

Code Block
bgColor#FFCCCC
langc
void set_flag(int number, int *sign_flag) {
  if (sign_flagNULL == NULLsign_flag)
    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;
}

...

Code Block
bgColor#ccccff
langc
#include <assert.h>
 
void set_flag(int number, int *sign_flag) {
  if (sign_flagNULL == NULLsign_flag)
    return;

  if (number >= 0) { /* account for number being 0 */
    *sign_flag = 1;
  }
  else {
    assert(number < 0);
    *sign_flag = *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;
}

...

Code Block
bgColor#FFCCCC
langc
#include <stdio.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 (-1 == do_auth()) {
    report_error("Unable to login");
  }
  return 0;
}

Noncompliant Code Example

In this noncompliant example,  the array elements a[n..2n] are uninitialized when they are accessed in the for loop:

Code Block
bgColor#ffcccc
langc
#include <stdlib.h>
 
void g(double *a, size_t n) {
  a = (double *)realloc(a, (n * 2 + 1) * sizeof(double));
  if (a != NULL) {
    for (size_t i = 0; i != n * 2 + 1; ++i) {
      if (a[i] < 0) {
        a[i] = -a[i];  /* violation */
      }
    }
 
    /* ... */
    free(a);
  }
}

Compliant Solution 

In this compliant example,  the array elements a[n..2n] are initialized to 0 when they are accessed in the for loop:

Code Block
bgColor#ccccff
langc
#include <stdlib.h>
 
void g(double *a, size_t n) {
  a = (double *)calloc(a, (n * 2 + 1) * sizeof(double));
  if (a != NULL) {
    for (size_t i = 0; i != n * 2 + 1; ++i) {
      if (a[i] < 0) {
        a[i] = -a[i]; 
      }
    }
 
    /* ... */
    free(a);
  }
}
}

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

Noncompliant Code Example

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

...

This solution is still problematic because 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-C. Use meaningful symbolic constants to represent literal values.)

Compliant Solution

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

Code Block
bgColor#ccccff
langc
#include <stdio.h>
 
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);
}

Compliant Solution

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

...

Code Block
bgColor#ccccff
langc
void f(const char *mbs) {
  size_t len;
  mbstate_t state;

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

  /* ... */
}

...