How can I solve heap-buffer-overflow in my C-code?

Question

I am fairly new to C-coding and I have a task where we run libFuzzer on a basic C-program to exploit problems and fix them. This is my C-program (it takes a string input and changes "&" to "&amp", ">" to "&gt" and "<" to "&lt"):

char *checkString(char str[50]) {
  int i;
  char *newstr;
  newstr = (char *)malloc(200);
  for (i = 0; i < strlen(str); i++) {
    if (str[i] == '&') {
      const char *ch = "&amp";
      strncat(newstr, ch, 4);
    } else if (str[i] == '<') {
      const char *ch = "&lt";
      strncat(newstr, ch, 3);
    } else if (str[i] == '>') {
      const char *ch = "&gt";
      strncat(newstr, ch, 3);
    } else {
      const char ch = str[i];
      strncat(newstr, &ch, 1);
    }
  }
  return newstr;
}

This is the error message from libFuzzer:

SUMMARY: AddressSanitizer: heap-buffer-overflow (/path/to/a.out+0x50dc14) in strncat

Anybody who knows how to possibly fix this heap buffer overflow problem? Thanks!

Answer 1

After newstr = (char *)malloc(200);, newstr is not yet properly initialized so you must not call strncat( newstr, ... ).
You can solve that e.g. by calling strcpy( newstr, "" ); after malloc() or by replacing malloc(200) with calloc(200,1) which fills the entire buffer with NUL.

Besides, as @stevesummit has mentioned, despite its declaration there is no guarantee, that strlen(str) < 50. So instead of allocating a fix number of 200 characters, you should alloc strlen(str)*4 + 1 ... or strlen(str)*5 + 1 if what you're doing is HTML esacping and you realize that & should be replaced by &