heap-buffer-overflow with fprintf

Question

I'm updating my question, very sorry for asking it the wrong way.

Now I could distill my problem into a single self-contained piece of code:

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

static __inline__ char* fileRead(char* file){
     FILE* fp;
     long fileSize;
     char* fileContents;

     fp = fopen ( file , "rb" );
     if(!fp){
          perror(file);
          exit(1);}

     /* this block writes the size of the file in fileSize */
     fseek( fp , 0L , SEEK_END);
     fileSize = ftell( fp );
     rewind( fp );

     /* allocate memory for entire content */
     fileContents = malloc(fileSize+1);
     if(!fileContents){
          fclose(fp);
          fputs("memory alloc fails",stderr);
          exit(1);}

     /* copy the file into the buffer */
     if(fread(fileContents, fileSize, 1, fp) != 1){
          fclose(fp);
          free(fileContents);
          fputs("entire read fails",stderr);
          exit(1);}

     /* close the file */
     fclose(fp);
     return fileContents;}

int main (){
     char* head10 = "";
     char* fileName = "testhtml.html";
     FILE* out = fopen(fileName, "w");

     head10 = fileRead("head10.html");
          printf("%s\n", head10);

     out = fopen(fileName, "wb");
          fprintf(out, "%s\n", head10);
          fclose(out);

     free(head10);
return 0;}

Here the head10.html file.

I'm compiling it with -fsanitize=address, and I'm getting an heap-buffer-overflow. The error seems to be caused at the line fprintf(out, "%s\n", head10);. head10 is the only malloc'd variable, so that makes sense.

I can print it without problems with printf, but when I try to write it to file with fprintf, an heap-buffer-overflow is generated.

===EDIT=== Looks like the problem came from using fprintf with a malloc'd var, as fprintf itself uses malloc under the hood, so the original alloc gets lost, and memory leaks.

So i rewrote my functions without using malloc:

#define _POSIX_C_SOURCE 200809L /* for getline() */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

static __inline__ void fileReset(char* fileName){
     FILE* out = fopen(fileName, "w");
     fwrite("" , sizeof(char) , strlen("") , out );
     fclose(out);}

static __inline__ void fileAppend(char* fileName, char* string){
     FILE* out = fopen(fileName, "a"); /* using "a" to APPEND */
     if(fwrite(string , sizeof(char) , strlen(string) , out ) != strlen(string)){
               printf("==file write error\n");
               exit(EXIT_FAILURE);}
     fclose(out);}

static __inline__ void fileAppendFile(char* source, char* dest){
     FILE* in = fopen(source, "r");
     char *line = NULL;
    size_t len = 0;
    size_t read;

     while ((read = getline(&line, &len, in)) != -1) {
          fileAppend(dest, line);}

        free(line);
        fclose(in);}

int main (){
     char* fileName = "testhtml.html";
     char* theme = "dark";

     fileReset(fileName);
     fileAppendFile("head10.html", fileName);
     fileAppend(fileName, theme);
return 0;}

Thanks a lot for all the help, very noob here, didn't know what -lasan was, now I know what an invaluable tool!

==EDIT-2== As pointed out by EmployedRussian, the problem in the original code was NOT fprintf, but the lack of a terminating '\0', look at their answer below, it does fix my original code :)

Answer 1

Looks like the problem came from using fprintf with a malloc'd var, as fprintf itself uses malloc under the hood, so the original alloc gets lost, and memory leaks.

I am afraid you learned the wrong lesson here.

While fprintf may indeed use malloc under the hood, your problem doesn't have anything to do with that.

I created a head10.html file containing abc\n (4 characters). Running your program with that input file produced:

==10173==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000015 at pc 0x7fb5db2c7054 bp 0x7ffd44e74de0 sp 0x7ffd44e74590
READ of size 6 at 0x602000000015 thread T0
    #0 0x7fb5db2c7053  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x4d053)
    #1 0x5654101dd435 in main /tmp/foo.c:43
    #2 0x7fb5db0dde0a in __libc_start_main ../csu/libc-start.c:308
    #3 0x5654101dd199 in _start (/tmp/a.out+0x1199)

0x602000000015 is located 0 bytes to the right of 5-byte region [0x602000000010,0x602000000015)
allocated by thread T0 here:
    #0 0x7fb5db381628 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107628)
    #1 0x5654101dd2db in fileRead /tmp/foo.c:20
    #2 0x5654101dd425 in main /tmp/foo.c:42
    #3 0x7fb5db0dde0a in __libc_start_main ../csu/libc-start.c:308

So the problem is that you allocated 5 bytes (as expected), but fprintf tried to read 6th character from that buffer.

Why would it do that? Because the format you used: %s expects to find a terminating NUL character (i.e. it expects a properly terminated C-string), and you gave it a pointer to non-terminated string with the following bytes:

a b c \n X

What value does the fifth byte contain? It's undefined (it came from malloc, and no value was written into it). Since that value is not NUL, fprintf tries to read the next (6th) byte, and that's when Address Sanitizer signals the error and aborts your program.

The correct fix is to NUL-terminate the string, like so:

 if (fread(fileContents, fileSize, 1, fp) != 1){ ... handle error
 fileContents[fileSize] = '\0';  // NUL-terminate the string.