CODEWITHSUNDEEP
cstringdynamicmallocbuffer-overflow
Akiyuu
Akiyuu

Reputation: 33

C: Heap Buffer Overflow when Storing Elements Using Dynamic Memory Allocation

I am writing a C program to store and then print out all strings given through stdin using dynamic memory allocation. It prompts the user for input until it reads "end of file" (EOF, ctrl + d). I am assuming that no line is longer than 255 characters.

I am relatively new to coding in C, and also new to using the malloc() function, so please bear with me if I am making some obvious, rookie mistake. Please offer advice and help me fix my program. :)

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define BUFFER 255

int main(void) 
{
    
    char line[BUFFER];
    int item = 0;
    char** data = malloc(sizeof(char*));
    
    for(;;)
    {
        printf("Enter string %d: ", item + 1);

        if (fgets(line, BUFFER, stdin) == NULL)
        {
            break;
        }

        else
        {
            fgets(line, BUFFER, stdin);
        }

        data = realloc(data, sizeof(void *) * (item + 1));
        data[item] = malloc(sizeof(char) * strlen(line));
        memcpy(data[item], line, strlen(line));

        item++;

    }
    
    printf("\n\nElements:\n");
    
    for(int j = 0; j < item; j++)
    {
        printf("%s",data[j]);

        free(data[j]);
    }

    free(data);

    return 0;
}

The first concern I have is that my program prompts the user twice to enter a string input per item increment. This is not supposed to happen, and should continually ask the user while updating item's count.

Also, my program does not store the strings entered correctly; it returns a buffer overflow message when attempting to access and print my stored strings. I have never seen an error quite like it before:

==20==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000052 at pc 0x55a8453c7865 bp 0x7ffd578edac0 sp 0x7ffd578ed280 READ of size 3 at 0x602000000052 thread T0 #0 0x55a8453c7864 in printf_common(void*, char const*, __va_list_tag*) asan_interceptors.cpp.o #1 0x55a8453c819a in __interceptor_vprintf (/home/run+0xb119a) (BuildId: abfaad925a09237bc199a1c2ac9a7d328268e42f) #2 0x55a8453c8277 in printf (/home/run+0xb1277) (BuildId: abfaad925a09237bc199a1c2ac9a7d328268e42f) #3 0x55a84543014d in main /home/main.c:39:9 #4 0x7f08c84fe28f in __libc_start_call_main /usr/src/debug/glibc/csu/../sysdeps/nptl/libc_start_call_main.h:58:16 #5 0x7f08c84fe349 in __libc_start_main /usr/src/debug/glibc/csu/../csu/libc-start.c:381:3 #6 0x55a845337084 in _start /build/glibc/src/glibc/csu/../sysdeps/x86_64/start.S:115

Upvotes: 0

Views: 254

Answers (1)

user3121023
user3121023

Reputation: 8286

getline is useful when the length of the longest line is unknown. It will allocate memory as needed. It also returns the number of characters processed.
It is good to reallocate to a temporary pointer. If realloc fails, the original pointer is still valid.

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

int main ( void) {

    char *line;
    size_t size = 0;
    ssize_t length = 0;
    int item = 0;
    char** data = NULL;

    for ( ; ; ) {
        printf ( "Enter string %d: ", item + 1);
        fflush ( stdout);

        if ( 0 > ( length = getline ( &line, &size, stdin))) {
            break;
        }

        char **temp = NULL;
        if ( NULL == ( temp = realloc ( data, sizeof *data * (item + 1)))) {
            fprintf ( stderr, "realloc problem\n");
            break;
        }
        data = temp;
        if ( NULL == ( data[item] = malloc ( sizeof **data * ( length + 1)))) {
            fprintf ( stderr, "malloc problem\n");
            break;
        }

        strcpy ( data[item], line);

        item++;
    }

    free ( line);

    printf("\n\nElements:\n");

    for(int j = 0; j < item; j++) {
        if ( data[j] && data[j][0]) { // not NULL and not zero
            printf ( "%s", data[j]);
        }
        free(data[j]);
    }
    free(data);

    return 0;
}

Upvotes: 1

Related Questions