strcpy()/strncpy() crashes on structure member with extra space when optimization is turned on on Unix?

Question

When writing a project, I ran into a strange issue.

This is the minimal code I managed to write to recreate the issue. I am intentionally storing an actual string in the place of something else, with enough space allocated.

// #include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdint.h>
#include <stddef.h> // For offsetof()

typedef struct _pack{
    // The type of `c` doesn't matter as long as it's inside of a struct.
    int64_t c;
} pack;

int main(){
    pack *p;
    char str[9] = "aaaaaaaa"; // Input
    size_t len = offsetof(pack, c) + (strlen(str) + 1);
    p = malloc(len);
    // Version 1: crash
        strcpy((char*)&(p->c), str);
    // Version 2: crash
        strncpy((char*)&(p->c), str, strlen(str)+1);
    // Version 3: works!
        memcpy((char*)&(p->c), str, strlen(str)+1);
    // puts((char*)&(p->c));
    free(p);
  return 0;
}

The above code is confusing me:

• With gcc/clang -O0, both strcpy() and memcpy() works on Linux/WSL, and the puts() below gives whatever I entered.
• With clang -O0 on OSX, the code crashes with strcpy().
• With gcc/clang -O2 or -O3 on Ubuntu/Fedora/WSL, the code crashes (!!) at strcpy(), while memcpy() works well.
• With gcc.exe on Windows, the code works well whatever the optimization level is.

Also I found some other traits of the code:

• (It looks like) the minimum input to reproduce the crash is 9 bytes (including zero terminator), or 1+sizeof(p->c). With that length (or longer) a crash is guaranteed (Dear me ...).

• Even if I allocate extra space (up to 1MB) in malloc(), it doesn't help. The above behaviors don't change at all.

• strncpy() behaves exactly the same, even with the correct length supplied to its 3rd argument.

• The pointer does not seem to matter. If structure member char *c is changed into long long c (or int64_t), the behavior remains the same. (Update: changed already).

• The crash message doesn't look regular. A lot of extra info is given along.

  

I tried all these compilers and they made no difference:

• GCC 5.4.0 (Ubuntu/Fedora/OS X/WSL, all are 64-bit)
• GCC 6.3.0 (Ubuntu only)
• GCC 7.2.0 (Android, norepro???) (This is the GCC from C4droid)
• Clang 5.0.0 (Ubuntu/OS X)
• MinGW GCC 6.3.0 (Windows 7/10, both x64)

Additionally, this custom string copy function, which looks exactly like the standard one, works well with any compiler configuration mentioned above:

char* my_strcpy(char *d, const char* s){
    char *r = d;
    while (*s){
        *(d++) = *(s++);
    }
    *d = '\0';
    return r;
}

Questions:

• Why does strcpy() fail? How can it?
• Why does it fail only if optimization is on?
• Why doesn't memcpy() fail regardless of -O level??

*If you want to discuss about struct member access violation, pleast head over here.

---

Part of objdump -d's output of a crashing executable (on WSL):

---

P.S. Initially I want to write a structure, the last item of which is a pointer to a dynamically allocated space (for a string). When I write the struct to file, I can't write the pointer. I must write the actual string. So I came up with this solution: force store a string in the place of a pointer.

Also please don't complain about gets(). I don't use it in my project, but the example code above only.

Answer 1

What you are doing is undefined behavior.

The compiler is allowed to assume that you will never use more than sizeof int64_t for the variable member int64_t c. So if you try to write more than sizeof int64_t(aka sizeof c) on c, you will have an out-of-bounds problem in your code. This is the case because sizeof "aaaaaaaa" > sizeof int64_t.

The point is, even if you allocate the correct memory size using malloc(), the compiler is allowed to assume you will never use more than sizeof int64_t in your strcpy() or memcpy() call. Because you send the address of c (aka int64_t c).

TL;DR: You are trying to copy 9 bytes to a type consisting of 8 bytes (we suppose that a byte is an octet). (From @Kcvin)

If you want something similar use flexible array members from C99:

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

typedef struct {
  size_t size;
  char str[];
} string;

int main(void) {
  char str[] = "aaaaaaaa";
  size_t len_str = strlen(str);
  string *p = malloc(sizeof *p + len_str + 1);
  if (!p) {
    return 1;
  }
  p->size = len_str;
  strcpy(p->str, str);
  puts(p->str);
  strncpy(p->str, str, len_str + 1);
  puts(p->str);
  memcpy(p->str, str, len_str + 1);
  puts(p->str);
  free(p);
}

Note: For standard quote please refer to this answer.

Answer 2

I reproduced this issue on my Ubuntu 16.10 and I found something interesting.

When compiled with gcc -O3 -o ./test ./test.c, the program will crash if the input is longer than 8 bytes.

After some reversing I found that GCC replaced strcpy with memcpy_chk, see this.

// decompile from IDA
int __cdecl main(int argc, const char **argv, const char **envp)
{
  int *v3; // rbx
  int v4; // edx
  unsigned int v5; // eax
  signed __int64 v6; // rbx
  char *v7; // rax
  void *v8; // r12
  const char *v9; // rax
  __int64 _0; // [rsp+0h] [rbp+0h]
  unsigned __int64 vars408; // [rsp+408h] [rbp+408h]

  vars408 = __readfsqword(0x28u);
  v3 = (int *)&_0;
  gets(&_0, argv, envp);
  do
  {
    v4 = *v3;
    ++v3;
    v5 = ~v4 & (v4 - 16843009) & 0x80808080;
  }
  while ( !v5 );
  if ( !((unsigned __int16)~(_WORD)v4 & (unsigned __int16)(v4 - 257) & 0x8080) )
    v5 >>= 16;
  if ( !((unsigned __int16)~(_WORD)v4 & (unsigned __int16)(v4 - 257) & 0x8080) )
    v3 = (int *)((char *)v3 + 2);
  v6 = (char *)v3 - __CFADD__((_BYTE)v5, (_BYTE)v5) - 3 - (char *)&_0; // strlen
  v7 = (char *)malloc(v6 + 9);
  v8 = v7;
  v9 = (const char *)_memcpy_chk(v7 + 8, &_0, v6 + 1, 8LL); // Forth argument is 8!!
  puts(v9);
  free(v8);
  return 0;
}

Your struct pack makes GCC believe that the element c is exactly 8 bytes long.

And memcpy_chk will fail if the copying length is larger than the forth argument!

So there are 2 solutions:

• Modify your structure

• Using compile options -D_FORTIFY_SOURCE=0(likes gcc test.c -O3 -D_FORTIFY_SOURCE=0 -o ./test) to turn off fortify functions.

  Caution: This will fully disable buffer overflow checking in the whole program!!