How to overcome from Stack Smashing error?

Question

I have a code like this, which is solved by me following a exercise problem from K&R book:

#include<stdio.h>
void stringCat(char *s,char *t)
{
    while(*s++);
    while((*s++ = *t++));
}
void main() 
{
    char message1[] = "hello whats been up?";
    int i;
    char message2[] = "this should be added at last";
    stringCat(message1,message2);
    for(i=0;i<50;i++)
    {
        printf("%c\n",message1[i]);
    }
}

The program works as intended to be and also I get output like this :

hello whats been up?this should be added at last

But I get an error followed by the output :

** stack smashing detected : ./a.out terminated ======= Backtrace: ========= */lib/tls/i686/cmov/libc.so.6(__fortify_fail+0x48)Aborted*

I came to know why does this occur from here. But I couldn't able to figure out why does this thing happens in my code?

I'm a newbie in C, I need your help. Thanks in advance.

Ry- · Accepted Answer · 2012-03-31T16:35:36.350

2

You have a buffer overflow. message1 only has enough space to store message1, not both itself and message2. You'll need to allocate a new char*:

char *stringCat(const char *s, const char *t)
{
    char *r = malloc(strlen(s) + strlen(t) + 1);
    char *p = r;

    while(*r++ = *s++);
    r--;
    while(*r++ = *t++);
    *r = '\0';

    return p;
}

void main() 
{
    char message1[] = "hello whats been up?";
    char message2[] = "this should be added at last";
    char *result = stringCat(message1, message2);

    printf("%s", result);

    free(result);
}

Here's a demo.

edited Mar 31 '12 at 16:35

answered Mar 31 '12 at 16:26

Ry-

218,210
55
464
476

Don't forget to `free` the buffer – Niklas B. Mar 31 '12 at 16:34
1

@NiklasB.: The program's ending anyway, but I'll put it there :) – Ry- Mar 31 '12 at 16:35
1

Yeah, but if it grows, proper memory management could get more important.. Thanks (: – Niklas B. Mar 31 '12 at 16:36

aib · Answer 2 · 2012-03-31T16:42:07.327

char arrays in C are not exactly strings. When you do

char hello[] = "Hello";

it is actually equivalent to

char hello[6] = { 'H', 'e', 'l', 'l', 'o', '\0' }

which means the array is barely large enough to hold the original string. Any data you read or write past the end will cause undefined behaviour, which typically manifests as stack smashing for writes past automatic arrays.

== How to overcome: ==

There are two options:

1) Make sure the destination string is long enough. It would be the caller's responsibility to do this. strcat() works this way.

1b) You could help the caller out by letting them supply the destination string's size and not writing past its end. strncat() works this way.

2) Allocate a long enough, third, destination string. It would be the caller's responsibility to deallocate (free) this string. @minitech has supplied an example of this. POSIX strdup() works in this manner.

score 1 · Answer 3 · answered Mar 31 '12 at 16:28

It happens simply because your StringCat function first increments s to the end of the string, and then continues to increase s into unknown lands while also increasing t.

void stringCat(char *s,char *t)
{
    while(*s++);
    while((*s++ = *t++));
}

You're trying to write t into s beyond where there's memory allocated.

You need to create a new memory space with the size of s + t + 1 and put your concatenated string in there.

How to overcome from Stack Smashing error?

3 Answers3