CODEWITHSUNDEEP
c++c++11structclangg++-4.7
user1710707
user1710707

Reputation: 21

Segfault when casting char array to struct

I'm trying to cast a char array to a struct and receiving a segfault when I compile with g++ 4.7 and run but not when I compile with clang 3.3 and run the program.

This is the message struct description:

typedef struct {
 char code[4];
 char nickname[24];
 struct in_addr addr;
 int port;
 int seqNum;
 char body[MESSAGE_MAX_LENGTH - (sizeof(char) * 28) - (sizeof(int) * 2) - sizeof(struct in_addr)];
} Message;

This is the line which is causing the segfault:

Message *recvMsg = (Message *)packet.buffer;

The packet object is of the following type:

class UDPPacket {
     public:
         static const size_t maxBufferSize = MESSAGE_MAX_LENGTH;
         uint8_t buffer[maxBufferSize];
         size_t length;
         struct in_addr addr;
         int port;

         UDPPacket();
         UDPPacket(struct in_addr addr, int port);
         std::string getRemoteAddrString();
         std::string getDataAsString();
 };

This is the backtrace produced by gdb:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff67f5700 (LWP 22753)]
0x00007ffff79787f7 in ?? () from /usr/lib64/libstdc++.so.6
(gdb) backtrace
#0  0x00007ffff79787f7 in ?? () from /usr/lib64/libstdc++.so.6
#1  0x00007ffff7978850 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() () from /usr/lib64/libstdc++.so.6
#2  0x0000000000411cb7 in SendMessage () at /home1/user123/ugh/final-project/main.cpp:189
#3  0x00000000004172df in std::_Bind_simple<void (*())()>::_M_invoke<>(std::_Index_tuple<>) (
    this=0x620310) at /usr/include/c++/4.7/functional:1598
#4  0x000000000041722f in std::_Bind_simple<void (*())()>::operator()() (this=0x620310)
    at /usr/include/c++/4.7/functional:1586
#5  0x00000000004171c8 in std::thread::_Impl<std::_Bind_simple<void (*())()> >::_M_run() (
    this=0x6202f8) at /usr/include/c++/4.7/thread:115
#6  0x00007ffff796f340 in ?? () from /usr/lib64/libstdc++.so.6
#7  0x00007ffff7bc6e0f in start_thread () from /lib64/libpthread.so.0
#8  0x00007ffff70e044d in clone () from /lib64/libc.so.6

I'm not sure why this is segfaulting when I compile with g++ but not with clang?

EDIT: This is what SendMessage() looks like:

 56 void SendMessage() {
 57     while(true){
 58         std::cout << "> ";
 59         std::string input;
 60         std::getline(std::cin, input);
 61
 62         if(input.length() > 140){
 63             std::cout << "[Message is too long and was not sent]" << std::endl;
 64             continue;
 65         }
 66
 67         Message sendMsg;
 68         ::strncpy(sendMsg.code, "SEND", 4);
 69         ::strncpy(sendMsg.body, input.c_str(), MESSAGE_MAX_LENGTH - 28);
 70
 71         if(isLeader){
 72             // Leader Code
 73             ::strncpy(sendMsg.nickname, lState->name.c_str(), lState->name.length());
 74             sendMsg.addr = lState->ownAddr.addr;
 75             sendMsg.port = lState->ownAddr.port;
 76
 77             sendMsg.seqNum = lState->seqNum;
 78             broadcast(lState->allClients, &sendMsg);
*lines 79 to 187 commented out*
187             lState->seqNum += 1;
188          }
189     } //end of while loop
190 }

Upvotes: 1

Views: 427

Answers (2)

paulsm4
paulsm4

Reputation: 121669

There's no guarantee the actual size of your struct is going to be the sum of the nominal sizes of each member of the struct.

The actual size could be LARGER due to padding or other compiler- and/or platform-specific issues.

Here are more details:

Why isn't sizeof for a struct equal to the sum of sizeof of each member?

Upvotes: 1

etheranger
etheranger

Reputation: 1273

The setup

Ignoring the fact that sizeof(Message) may not be what you think it is (though it may be!), you're putting Message sendMsg on the stack.

Assuming standard type widths, sendMsg.body has space for [MESSAGE_MAX_LENGTH - 36 - sizeof(in_addr)] chars.

The problem

Then you're coming along with ::strncpy(sendMsg.body, input.c_str(), MESSAGE_MAX_LENGTH - 28); which potentially writes a lot more characters than there's space for! This can keep writing past the end of sendMsg and whatever happens to be next to it on the stack, which can vary between compilers. Maybe your clang compilation & input are getting lucky (though still incorrect).

Once you've overwritten innocent stack variables, Bad Things(tm) like segfaults can happen at any time.

The prevention

Assuming you don't need your char arrays null-terminated (which you might), make sure you never copy more data than you have room for.

::strncpy(sendMsg.code, "SEND", sizeof(sendMsg.code));
::strncpy(sendMsg.body, input.c_str(), sizeof(sendMsg.body));
::strncpy(sendMsg.nickname, lState->name.c_str(), sizeof(sendMsg.nickname));

Bear in mind this may still be discarding information and not null-terminating, but at least it won't stomp your stack.

Upvotes: 0

Related Questions