Jump to content
  • Advertisement
Sign in to follow this  
Tispe

WinSock, identifying a packet

This topic is 2771 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Yes, it could. It could look like mine =P

More seriously, use memcpy() or std::copy() for non-character data. Also there was a reason I separated the ChatHeader into a separate struct from the character data.

Share this post


Link to post
Share on other sites
Advertisement
warning C4200: nonstandard extension used : zero-sized array in struct/union Cannot generate copy-ctor or copy-assignment operator when UDT contains a zero-sized array

warning C4996: 'std::copy': Function call with parameters that may be unsafe - this call relies on the caller to check that the passed values are correct.

something to be worried about?

Share this post


Link to post
Share on other sites
You can change the zero sized array to 1 character, or to some arbitrary limit you're happy to set. You'd need to double check to ensure you are using the right sizes in all locations (sizeof() for allocation, the logical size for send/recv).

I used std::copy in a safe way. That warning there is a non standard attempt by Microsoft to deprecate some of the standard APIs. Whether that concerns you is up to you, but it doesn't bother me.

Share this post


Link to post
Share on other sites
when you copy from a string to Char buffer, will you corrupt the data? Or does a nul terminated char buffer look at same as a string in memory?


struct ChatHeader
{
unsigned int ID, recipient, tag;
};

struct ChatPayload
{
ChatHeader header;
char text[4];
};

void SendChatPacket(unsigned int recipient, unsigned int tag, const std::string &text)
{
ChatPayload *chat = reinterpret_cast<ChatPayload *>(std::malloc(sizeof(ChatHeader) + text.length()));
if(!chat)
{
// Error handling...
}
chat->header.ID = 1;
chat->header.recipient = recipient;
chat->header.tag = tag;
// chat->header.TextLen = text.length() if you wish
std::copy(text.begin(), text.end(), chat->text);

const char *data = reinterpret_cast<const char *>(chat);
// Send code here

std::free(chat);
}

Share this post


Link to post
Share on other sites
I'm copying using the string's iterators, so there is no problem provided there is enough room. The representation of std::string is implementation defined, but to meet the standard specification there is generally a buffer of contiguous characters somewhere. They aren't necessarily NUL terminated though, the standard allows them not to be. The implementation can be lazy about NUL terminating them, in case you rarely call c_str().

Instead of std::copy() you could use strcpy(chat->text, text.c_str()) if you are more comfortable with such functions, or some buffer safe variant of strcpy() if you wish.

Share this post


Link to post
Share on other sites

when you copy from a string to Char buffer, will you corrupt the data? Or does a nul terminated char buffer look at same as a string in memory?


You should not assume that the data in your chat packet is actually zero terminated, because an attacker may easily create a packet that says it's a chat packet, but doesn't zero-terminate the buffer.
Thus, all variable-length fields in a network protocol should generally be length-prefixed rather than use a terminator.
Additionally, you should verify that the length doesn't overflow the actual data you have in the packet -- again, because otherwise, an attacker could read your stack/heap by crafting a malformed packet.

Share this post


Link to post
Share on other sites
I'm quite pleased with this code


struct ChatHeader
{
unsigned int ID, recipient, tag;
};

void SendChatPacket(unsigned int recipient, unsigned int tag, const std::string &text)
{
char * buffer = (char*)malloc(sizeof(ChatHeader) + text.length());

ChatHeader* chat = reinterpret_cast<ChatHeader*>(buffer);
chat->ID = 1;
chat->recipient = recipient;
chat->tag = tag;

memcpy(buffer+sizeof(ChatHeader),text.c_str(), text.length());

//sendto(Socket, Buffer, sizeof(ChatHeader)+text.length(), 0, (sockaddr*)&RemoteAddress, sizeof(sockaddr));

free(buffer);
}

Share this post


Link to post
Share on other sites

I'm quite pleased with this code



That contains neither a length of the string nor a zero termination. While you're sending the packet as a single UDP packet, in a real system, you'd put many messages into a single UDP packet, and wrap it in framing (to allow for re-sending dropped messages, etc), and at that point you can't know the length of the text without actually encoding it somehow.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!