Do I risk a buffer overrun here and how would I avoid it?

Question

I implemented an easy list structure where single list elements are defined by the following structure:

struct list_elem {
    struct list_elem *next;  // ptr to the next element
    char             *data;  // ptr to data
};

Now, I want to do the following:

struct list_elem *elem;
int main() {
    elem->data = "some_string";
    strcat(elem->data, "another_string");
}

I am worried about an overrun because the man page of strcat states:

The char *strcat(char *dest, const char *src) function appends the src string to the dest string, overwriting the terminating null byte ('\0') at the end of dest, and then adds a terminating null byte. The strings may not overlap, and the dest string must have enough space for the result. If dest is not large enough, program behavior is unpredictable; buffer overruns are a favorite avenue for attacking secure programs.

And basically I got no idea how much memory is allocated for my list element.

Answer 1

This statement:

elem->data = "some_string";

makes the data pointer point to string literal "some_string".
And here:

strcat(elem->data, "another_string");

you are trying to copy string literal "another_string" to a pointer which is pointing to another string literal. As per the standard attempting to modify a string literal results in undefined behavior because it may be stored in read-only storage.

You should allocate memory to data, like this:

elem->data = calloc(50, 1); // sizeof (char) is 1

Then copy the "some_string" to it;

strcpy (elem->data, "some_string");

Then concatenate "another_string" to it:

strcat (elem->data, "another_string");

Alternatively, you can use snprintf() also:

snprintf (elem->data, 49, "%s%s", "some_string", "another_string");

EDIT:

Thanks @alk for pointing this.

The elem pointer is not pointing to valid memory. You should first allocate memory to the struct list_elem pointer elem, like this:

elem = malloc (sizeof (struct list_elem));
if (elem == NULL)
    exit(EXIT_FAILURE);

Answer 2

You can search for astrxxx functions(,which are not standard C functions). They dynamically allocates memories while operation. Github example implementations

• astrcat is implemented above.
• asprintf is a dynamic version of sprintf.
• and more are provided by some GNU compilers

Note: you SHOULD FREE dynamically allocated memories!!

Plus, you should use it like this:

struct list_elem elem;    //removed *, as it can cause seg fault if not initialized. you have to initialize by using struct list_elem * elem=malloc(sizeof(struct list_elem)); or something.
int main() {
    elem.data = strdup("some_string");//you must free data later
    astrcat(&elem.data, "another_string");
    //use it
    free(elem.data);
}

First, the struct list_elem* elem is initialized as NULL, so it has to be initialized with valid address before -> statements. You can assign a pointer to the data section to the data, which cannot be modified normally.