[Bug c/91205] New: -fstack-protector-strong -D_FORTIFY_SOURCE=2 breaks tftpd

[ricardo at ribalda dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91205

Bug ID: 91205
   Summary: -fstack-protector-strong -D_FORTIFY_SOURCE=2 breaks
tftpd
   Product: gcc
   Version: 9.1.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ricardo at ribalda dot com
  Target Milestone: ---

Current GNU inetutils tftpd breaks at runtime with current gcc (also tested
with 8.3).

The code throws a:

*** buffer overflow detected ***: 
Aborted

on its error path. The error seems to be due to the way the arpa/tftp.h headers
are handled.

The simplest testcase that I have managed to craft is:

//compile with: CFLAGS=-fstack-protector-strong -D_FORTIFY_SOURCE=2 -Wformat
-Wformat-security -Werror=format-security -O2
#include 
#include 

struct  tftphdr {
short   th_opcode;  /* packet type */
union {
chartu_padding[3];  /* sizeof() compat */
struct {
char tu_data[0];/* data or error string */
} __attribute__ ((__packed__)) th_u2;
} __attribute__ ((__packed__)) th_u1;
} __attribute__ ((__packed__));

static char buf[512];

int main(int argc, char* argv[])
{
struct tftphdr *tp;

tp = (struct tftphdr *) buf;
strcpy(tp->th_u1.th_u2.tu_data, "Hello world");

fprintf(stdout, "Code works!\n");

return 0;
}


Although this other testcase is closer to the original source:

#include 
#include 
#include 
#include 


/* Some systems define PKTSIZE in .  */
#ifndef PKTSIZE
#define PKTSIZE SEGSIZE+4
#endif


static char buf[PKTSIZE];

struct errmsg
{
  int e_code;
  const char *e_msg;
} errmsgs[] =
  {
{EUNDEF, "Undefined error code"},
{ENOTFOUND, "File not found"},
{EACCESS, "Access violation"},
{ENOSPACE, "Disk full or allocation exceeded"},
{EBADOP, "Illegal TFTP operation"},
{EBADID, "Unknown transfer ID"},
{EEXISTS, "File already exists"},
{ENOUSER, "No such user"},
{-1, 0}
  };


static void nak (int error)
{
register struct tftphdr *tp;
int length;
register struct errmsg *pe = &errmsgs[error];

tp = (struct tftphdr *) buf;
tp->th_opcode = htons ((unsigned short) ERROR);
tp->th_code = htons (pe->e_code);
strcpy(tp->th_msg, pe->e_msg);
length = strlen (pe->e_msg);
length += 5;
}

int main(int argc, char* argv[])
{
int i;

for (i=0;i<8;i++)
nak(i);

fprintf(stdout, "Code works!\n");

return 0;
}


Replacing strcpy with memcpy solves the issue. A patch has been sent to
inetutils. https://www.mail-archive.com/bug-inetutils@gnu.org/msg03036.html

[Bug middle-end/91205] -fstack-protector-strong -D_FORTIFY_SOURCE=2 breaks tftpd

[ricardo at ribalda dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91205

--- Comment #2 from Ricardo Ribalda  ---
Hi Jakub

I can agree that I do not like the code style and I have already send a patch
for that.

On the other hand is a bit frustrating that mangling a bit the struct, or using
memcpy is enough to make it work.:


#include 
#include 

struct  tftphdr {
short   th_opcode;  /* packet type */
union {
chartu_padding[3];  /* sizeof() compat */

char tu_data[0];/* data or error string */

} __attribute__ ((__packed__)) th_u1;
} __attribute__ ((__packed__));

static char buf[512];

int main(int argc, char* argv[])
{
struct tftphdr *tp;

tp = (struct tftphdr *) buf;
strcpy(tp->th_u1.tu_data, "Hello world");

fprintf(stdout, "Code works %s!\n", tp->th_u1.tu_data);

return 0;
}

[Bug middle-end/91205] -fstack-protector-strong -D_FORTIFY_SOURCE=2 breaks tftpd

[ricardo at ribalda dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91205

--- Comment #3 from Ricardo Ribalda  ---
(and thanks for your fast response  :) )

[Bug middle-end/91205] -fstack-protector-strong -D_FORTIFY_SOURCE=2 breaks tftpd

[ricardo at ribalda dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91205

--- Comment #5 from Ricardo Ribalda  ---
Thanks for the clarification!


(And now, after using C/gcc daily for over 18 years I realise that I have no
fucking clue about C and gcc :) )