Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

In this noncompliant code example, the dynamically allocated buffer referenced by p overflows for values of n > INT_MAX.

Code Block
bgColor#FFcccc
langc
char *copy(size_t n, const char *str) {
  int i;
  char *p;

  if (n == 0) {
    /* Handle unreasonable object size error */
  }
  p = (char *)malloc(n);
  if (p == NULL) {
    /* Handle malloc failure */
  }
  for ( i = 0; i < n; ++i ) {
    p[i] = *str++;
  }
  return p;
}

/* ... */

char str[] = "hi there";
char *p = copy(sizeof(str), str);

...

Declaring i to be of type rsize_t eliminates the possible integer overflow condition (in this example). Also, the argument n is changed to be of type rsize_t to document additional validation in the form of a check against RSIZE_MAX.

Code Block
bgColor#ccccff
langc
char *copy(rsize_t n, const char *str) {
  rsize_t i;
  char *p;

  if (n == 0 || n > RSIZE_MAX) {
    /* Handle unreasonable object size error */
  }
  p = (char *)malloc(n);
  if (p == NULL) {
    /* Handle malloc failure */
  }
  for (i = 0; i < n; ++i) {
    p[i] = *str++;
  }
  return p;
}

/* ... */

char str[] = "hi there";
char *p = copy(sizeof(str), str);

...

In this noncompliant code example, the value of length is read from a network connection and passed as an argument to a wrapper to malloc() to allocate the appropriate data block. Provided that the size of an unsigned long is equal to the size of an unsigned int, and both sizes are equal to or smaller than the size of size_t, this code runs as expected. However, if the size of an unsigned long is greater than the size of an unsigned int, the value stored in length may be truncated when passed as an argument to alloc().

Code Block
bgColor#FFcccc
langc
void *alloc(unsigned int blocksize) {
  return malloc(blocksize);
}

int read_counted_string(int fd) {
  unsigned long length;
  unsigned char *data;

  if (read_integer_from_network(fd, &length) < 0) {
    return -1;
  }

  data = (unsigned char*)alloc(length);
  if (data == NULL) {
    /* Handle error */
  }

  if (read_network_data(fd, data, length) < 0) {
    free(data);
    return -1;
  }
  data[length-1] = '\0';

  /* ... */
  free( data);
  return 0;
}

...

Declaring both length and the blocksize argument to alloc() as rsize_t eliminates the possibility of truncation. This compliant solution assumes that read_integer_from_network() and read_network_data() can also be modified to accept a length argument of type pointer to rsize_t and rsize_t, respectively. If these functions are part of an external library that cannot be updated, care must be taken when casting length into an unsigned long to ensure that integer truncation does not occur.

Code Block
bgColor#ccccff
langc
void *alloc(rsize_t blocksize) {
  if (blocksize == 0 || blocksize > RSIZE_MAX) {
    /* Handle error */
  }
  return malloc(blocksize);
}

int read_counted_string(int fd) {
  rsize_t length;
  unsigned char *data;

  if (read_integer_from_network(fd, &length) < 0) {
    return -1;
  }

  data = (unsigned char*)alloc(length);
  if (data == NULL) {
    /* Handle error */
  }

  if (read_network_data(fd, data, length) < 0) {
    free(data);
    return -1;
  }
  data[length-1] = '\0';

  /* ... */
  free( data);
  return 0;
}

...