CODEWITHSUNDEEP
c
Steve
Steve

Reputation: 8293

Crash using strcpy into an offset of a data structure

I have a particular data structure used in a C program which I am using to attach type information to values. A simple example looks like the following:

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

struct teststruct
{
    char type;
    union {
        char s;
        int i;
        float f;
    } value;
};

int main()
{
    struct teststruct *t = malloc(sizeof(struct teststruct)+10);
    t->type = 'i';
    t->value.i = 42;
    free(t);

    t = malloc(sizeof(struct teststruct)+10);
    t->type = 's';
    strcpy(&t->value.s, "test");

    free(t);

    printf("Finished without errors.\n");
}

As you can see, my intention is to use the type field to identify the value's type, and use the value field to contain a union of the possible values. When the data is a string, the idea is to allocate more memory than sizeof(struct teststruct) and then access the string with &t->value.s.

Although this works, it is apparently problematic for the optimiser. Using gcc version 4.7.2, I get the following in non-optimised conditions:

$ gcc -O0 -o test test.c
$ ./test 
Finished without errors.

No problem. However, under the optimiser, it gives me a warning:

$ gcc -O2 -o test test.c
In file included from /usr/include/string.h:642:0,
                 from test.c:4:
In function ‘strcpy’,
    inlined from ‘main’ at test.c:25:15:
/usr/include/i386-linux-gnu/bits/string3.h:105:3: warning: call to __builtin___memcpy_chk will always overflow destination buffer [enabled by default]

And indeed,

$ ./test 
*** buffer overflow detected ***: ./test terminated

However, if I replace strcpy with memcpy, this works fine, and it also works well if I replace strcpy with a for-loop. However, strncpy crashes just like strcpy. I am absolutely not overwritten outside of malloc'ed memory, so I don't know why this is crashing.

I realize that copying into a weird offset of the data structure is not so usual, so the question is, am I violating some semantic contract of strcpy, or is this a bug in the compiler?

Thanks.

Upvotes: 3

Views: 4852

Answers (2)

Ezequiel Garcia
Ezequiel Garcia

Reputation: 1057

I'm no gcc expert myself, but after reading [1], I believe gcc -O2 is implementing that strcpy() call with a memcpy, and using __builtin___memcpy_chk to check for overflows.

According to the example in [1] the reasons for __builtin___memcpy_chk is apparently to check the destination object length, and allow to catch buffer overflows at runtime.

And also as per [1], you'll get a compile-time warning if gcc knows at compile time that __builtin___memcpy_chk itself will overflow.

If you still wish to retain your obscure char array allocation, I'd say you should at least use a safe strcpy:

strncpy(&t->value.s, "test", 9);

Notice that I can't trigger this behavior on my machine, and thus I can't disassemble the code to take a look at what's really going on.

[1] https://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html

Upvotes: 0

John3136
John3136

Reputation: 29266

Your code is not valid here:

strcpy(&t->value.s, "test");
// value.s is a single char, not a string that you can store
// an arbitrary number of characters into.
//

You either need to give s some space

struct teststruct
{
    char type;
    union {
        char s[10]; // length depends on your specific needs
        int i;
        float f;
    } value;
};

OR make s a pointer and dynamically allocate as required

struct teststruct
{
    char type;
    union {
        char *s;
        int i;
        float f;
    } value;
};
// instead of strcpy(&t->value.s, "test"); use t->value.s = strdup("test")
// don't forget to free the space when you are done.

Upvotes: 2

Related Questions