why do I get heap-buffer-overflow in C?

Question

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

int main(int argc, char* argv[])
{
  if (argc == 1)
    exit(EXIT_FAILURE);

  int count = 2;

  int i, j;
  for (i = 1; i < argc; i++)
  {
    for (j = 0; j < strlen(argv[i]); j++)
      count++;
  }

  printf("%d\n", count);

  char* original = malloc(sizeof(char) * count);
  printf("%p\n", original);
  char* copy = malloc(sizeof(char) * count);
  printf("%p\n", copy);
  memset(original, 0, strlen(original));
  memset(copy, 0, strlen(copy));

  strcpy(original, argv[1]);
  for (i = 2; i < argc; i++)
  {

    strcat(original, argv[i]);

  }

  int coun = 0;
  for (i = 0; i < strlen(original); i++)
  {
    if (original[i] == '(' || original[i] == '{' || original[i] == '[' ||
      original[i] == ')' || original[i] == '}' || original[i] == ']')
    {
      copy[coun] = original[i];
      coun++;
    }
  }

  printf("%s\n", original);
  printf("%s\n", copy);

  free(original);
  free(copy);
  exit(EXIT_SUCCESS);
}

I used gcc -Wall -Werror -fsanitize=address balance.c -o balance to make file and ./balance '((' to test

and I got this message

enter image description here

what is the problem?

It is the code to get argv's contents and get only parenthesis on a string.

It might be error checking on -fsanitize=address I got it but I cannot find any errors on my code, so can someone check this please?

strlen(original) - what do you think it is supposed to return? original is not a string at all. Same with strlen(copy) — Eugene Sh.
– Eugene Sh., Commented Feb 19, 2021 at 16:01
string in C is not a type. It is a content. That is an array terminated by a null-character. Your arrays are uninitialized. — Eugene Sh.
– Eugene Sh., Commented Feb 19, 2021 at 16:03
I told you what your problem is. Do not use strxxxx functions on something which is not a string. You already know the sizes of the buffers though (sizeof(char) * count), you used them for malloc, so use the same for memsets. — Eugene Sh.
– Eugene Sh., Commented Feb 19, 2021 at 16:10

dbush · Accepted Answer · 2021-02-19 16:12:44Z

4

Your problem is here:

memset(original,0,strlen(original));
memset(copy,0,strlen(copy));

Both original and copy point to memory returned by malloc which is uninitialized. The strlen function reads the bytes pointed to by its argument until it finds a byte with value 0. This means that 1) you're reading uninitialized memory, and 2) because the contents are indeterminate the function could read past the end of allocated memory. Both of these actions trigger undefined behavior.

You know that both memory locations point to count bytes, so pass that to memset:

memset(original,0,count);
memset(copy,0,count);

Better yet, use calloc instead of malloc which returns memory that has been initialized to 0:

char* original = calloc(sizeof(char), count);
char* copy = calloc(sizeof(char), count);

answered Feb 19, 2021 at 16:12

dbush

233k27 gold badges261 silver badges334 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Roman Pavelka · Accepted Answer · 2021-02-19 16:22:12Z

1

Here is the problem:

...
  char* original = malloc(sizeof(char) * count);
...
  memset(original, 0, strlen(original));
...

strlen is counting how many characters are there before encountering null byte \0. The memory allocated by malloc is uninitialized so you are not guaranteed to find any null byte before accessing memory you are not allowed to.

answered Feb 19, 2021 at 16:22

Roman Pavelka

4,2512 gold badges15 silver badges33 bronze badges

Collectives™ on Stack Overflow

why do I get heap-buffer-overflow in C?

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related