Why is using memcpy with custom structs that have pointers in them causing a heap buffer overflow?

Question

I am making a program which requires multiple types of dynamic arrays in C89. What I am trying to do is create my own implementation of a dynamic array that supports custom struct data.

However, while using my dynamic array implementation, I get a heap buffer overflow and I cannot figure out why. I appear to have no casting issues. I have properly allocated and resized the dynamic array. And I used appropriate sizing. I should be able to add the struct to the array with no problems.

Here is the main C file containing the dynamic array and point struct. The heap buffer overflow is occurring at dynamic_array_push. Note: For testing purposes, I created a point_a variable, so there is no actual purpose of making a dynamic array in this instance.

struct point {
  int x;
  int y;
  const char* id;
};

int main() {
  struct dynamic_array points = dynamic_array_create(sizeof(struct point));
  struct point point_a = {1, 1, "point_a"};
  size_t i;

  dynamic_array_push(&points, (void*)&point_a); /* Heap buffer overflow here */

  return 0;
}

Here is the dynamic array header file which contains the minimal functionality for creating and pushing to the dynamic array (excludes freeing memory and popping). And I have isolated the cause of the heap buffer overflow, with it occurring at memcpy.

#define DYNAMIC_STARTING_SIZE 1
#define DYNAMIC_GROWTH_AMOUNT 1.5

struct dynamic_array {
  void* data;
  size_t amount;
  size_t capacity;
  size_t element_size;
};

struct dynamic_array dynamic_array_create(size_t element_size) {
  struct dynamic_array dynamic_array;

  dynamic_array.data = malloc(sizeof(DYNAMIC_STARTING_SIZE * element_size));
  dynamic_array.amount = 0;
  dynamic_array.capacity = DYNAMIC_STARTING_SIZE;
  dynamic_array.element_size = element_size;

  return dynamic_array;
}

void dynamic_array_push(struct dynamic_array* dynamic_array,
                        const void* element) {
  unsigned char* destination =
      (unsigned char*)dynamic_array->data +
      dynamic_array->amount * dynamic_array->element_size;

  if (dynamic_array->amount >= dynamic_array->capacity) {
    dynamic_array->capacity =
        (size_t)ceil(dynamic_array->capacity * DYNAMIC_GROWTH_AMOUNT);
    dynamic_array->data =
        realloc(dynamic_array->data,
                dynamic_array->capacity * dynamic_array->element_size);
  }

  memcpy((void*)destination, element, dynamic_array->element_size); /* Heap buffer overflow here */

  dynamic_array->amount++;
}

I printed out the values of the memory address locations and they are what they should be for the size of point_a and got these values on my machine:

Starting memory address: 0x502000000010
Destination memory address: 0x502000000010
Address after array data: 0x502000000020

But the address sanitizer is showing a write size of 16 bytes (as const char* point_a is 8 bytes and two ints also being 8 bytes on my machine) at address 0x502000000018.

Then, I just decided to remove the const char* data field, which fixed the issue. I then tested and figured out that any pointer, no matter what position it is in the struct, causes the heap overflow.

The destination should be correct, as I am getting the data array location, and adding amount times the element size. The element and dynamic_array->element_size is in the correct locations. I am also appropriately allocating memory to the array data when you first create the array as well as reallocating when needed (which does not matter in this context as the array already can hold one value once created).

Is there something else I should be doing to support pointers in the struct that is being used for the dynamic array or is something else wrong here?

Questions that are similar to my specific problem which I failed to get an answer from:

• Having trouble diagnosing a heap buffer overflow
• Why is address-sanitizer causing a heap-buffer-overflow error in my multiplication matrix?

Other similar questions about memcpy and dynamic arrays:

• Using memcpy to copy a structure into the heap
• C dynamically growing array
• memory copy dynamically allocated array
• Dynamic array of pointers to structs

Answer 1

Your main problem is where you create the initial array:

dynamic_array.data = malloc(sizeof(DYNAMIC_STARTING_SIZE * element_size));

You're asking for the size of the datatype for the expression DYNAMIC_STARTING_SIZE * element_size, i.e. size_t, instead of the value of the expression. A size_t will most likely be 8 bytes, so an element size larger than this won't be allocating enough space. This results in you writing past the end of allocated memory.

You instead want:

dynamic_array.data = malloc(DYNAMIC_STARTING_SIZE * element_size);

You've also got a problem here:

  unsigned char* destination =
      (unsigned char*)dynamic_array->data +
      dynamic_array->amount * dynamic_array->element_size;

  if (dynamic_array->amount >= dynamic_array->capacity) {
    dynamic_array->capacity =
        (size_t)ceil(dynamic_array->capacity * DYNAMIC_GROWTH_AMOUNT);
    dynamic_array->data =
        realloc(dynamic_array->data,
                dynamic_array->capacity * dynamic_array->element_size);
  }

If the reallocation moves the memory, then destination will be pointing to an invalid location. So do the size check first:

  if (dynamic_array->amount >= dynamic_array->capacity) {
    dynamic_array->capacity =
        (size_t)ceil(dynamic_array->capacity * DYNAMIC_GROWTH_AMOUNT);
    dynamic_array->data =
        realloc(dynamic_array->data,
                dynamic_array->capacity * dynamic_array->element_size);
  }

  unsigned char* destination =
      (unsigned char*)dynamic_array->data +
      dynamic_array->amount * dynamic_array->element_size;