Boost ASIO - Invalid Header and Body for TCP packet

Question

I am using Boost ASIO as a TCP network communication solution for my project.

Here is my code:

Client.h:

namespace Vibranium{
    class Client: public std::enable_shared_from_this<Client>
    {
    public:
        Client(tcp::socket socket)
        : socket(std::move(socket))
        {
        }
        void start();
        int connectionId;
        tcp::socket socket;
        void Send(ServerOpcode serverOpcode, const std::string& message);

    private:
        void read_header();
        void read_body();
        Packet _packet;
    };
}
#endif //VIBRANIUM_CORE_CLIENT_H

Here is how I read the header and the body:

void Vibranium::Client::read_header() {
    auto self(shared_from_this());
    boost::asio::async_read(socket,
    boost::asio::buffer(_packet.data_, _packet.header_length),
    [this, self](boost::system::error_code ec, std::size_t /*length*/)
    {
        if (!ec)
        {
            std::cout << "Header: " << std::endl;
            std::cout.write(_packet.data_, _packet.header_length);
            std::cout << "\n";
            read_body();
        }
        else
        {
            std::cerr << "Invalid header sent!" << std::endl;
        }
    });
}


void Vibranium::Client::read_body() {
    auto self(shared_from_this());
    socket.async_read_some(boost::asio::buffer(_packet.data_, _packet.body_length),
   [this, self](boost::system::error_code ec, std::size_t length)
   {
       if ((boost::asio::error::eof == ec) || (boost::asio::error::connection_reset == ec))
       {
           Logger::Log("Disconnected ID: " + std::to_string(connectionId),Logger::Error, true);
           for (int i = 0; i < Server::Clients.size(); ++i) {
               if(Server::Clients[i]->connectionId == connectionId)
                   Server::Clients.erase(Server::Clients.begin()+i);
           }
       }
       else
       {
           std::cout << "Body: " << std::endl;
           std::cout.write(_packet.data_, _packet.body_length);
           std::cout << "\n";
           //Send(ServerOpcode::SMSG_AUTH_CONNECTION_RESPONSE,"How are you, mate?");
           read_header();
       }
   });
}

Here is how I sent a message with header and body:

Config config("AuthServer");
std::string defaultIP   = "127.0.0.1";
std::string defaultPort = "8080";
int connectionsNumber   = CommandQuestion<int>::AskQuestion("How many connections do you want established?");
std::cout << "Initializing " << std::to_string(connectionsNumber) << " connection/s." << std::endl;

std::cout << "Trying to connect to  " <<  defaultIP << " on port: " << config.GetConfigValue("AuthServerPort", defaultPort)  << std::endl;
boost::asio::io_context io_context;
std::vector<tcp::socket> sockets;
for (int i = 0; i < connectionsNumber; ++i) {
    try
    {
        sockets.emplace_back(io_context);
        tcp::socket& s{sockets.back()};
        tcp::resolver resolver(io_context);
        boost::asio::connect(s, resolver.resolve( defaultIP,config.GetConfigValue("AuthServerPort", defaultPort)));

        enum { body_length = 1024 };
        enum { header_length = 8 };
        enum { max_length = body_length +  header_length};
        char header_[header_length];
        char body_[body_length];
        char data_[header_length + body_length];

        ServerOpcode opc;
        opc = ServerOpcode::SMSG_AUTH_CONNECTION_RESPONSE;
        std::string message = "I am testing here!!!";


        snprintf(header_,header_length,"%x\n",opc);
        strcpy(body_, message.c_str());
        sprintf(data_, "%s %s", header_, body_);

        size_t request_length = sizeof(data_)/sizeof(*data_);

        std::cout << "header: " << header_ << std::endl;
        std::cout << "body: " << body_ << std::endl;
        std::cout << "whole message is: " << data_ << std::endl;
        std::cout << "size: " << request_length << std::endl;
        std::cout << "max size: " << max_length << std::endl;

        boost::asio::write(s, boost::asio::buffer(data_, request_length));
    }
    catch (std::exception& e)
    {
        std::cerr << "Exception: " << e.what() << "\n";
    }

}

Here is the output on the server:

New Connection (ID: 1)
Header: 
1
 I am 
Body: 
testing here!!!
hP��U��TK���U�U�� K�h
�*�� K�) �J�Uh
 0�y���x������������RK�Px���� K��RK�`x��\TUK��TK��;�#K��TJ�d�"K��XUK�v\TUK� �TK�{�|@X#K� �TJ�`FK��XUK��\TUK� �TK��G�|X^#K� �TJ�4AK��XUK�:\TUK��TK�z���M$K��TJ���#K��XUK��
u��u�z��x�TK��u��<}(K�@u��Pu���aUK��TJ��TK������TJ��TK�x�TK���U�{��������UK��M$K��TK����@K��{����UK�
Invalid header sent!

Here is the output on the client:

header: 1

body: I am testing here!!!
whole message is: 1
 I am testing here!!!
size: 1032
max size: 1032

Process finished with exit code 15

In order not to make this question extremely over-flooded with code here is the contents of Packet.h: https://pastebin.com/cnNzpRpV and Packet.cpp: https://pastebin.com/mbZPxf4e

What I can see is that somehow both header and body got transferred however they are not parsed right. Why am I receiving such unformed output on the server? Where is my mistake and how can I fix it?

Answer 1

Analysis

Let us step through the code to understand what is happening. In your client, you are initializing a buffer of size 8+1024, you fill some data into this:

char data_[header_length + body_length];
...
sprintf(data_, "%s %s", header_, body_);

You can see that the data that is written into data_ is smaller than the entire buffer size. Since you did not zero-initialize data_, the remaining part of it will be filled with random data (whatever happend to be at that position on the stack previously). The string "%s %s" is a zero-terminated string meaning that in memory it will be followed by a closing zero-byte. So when sprintf inserts the header and body into this formatting string, you will again obtain a zero-terminated string that is written to data_. So the data_ buffer will contain:

[--- header ---] <space> [--- body---] <zero-byte> [--- random data ---]

Also note that the header and body in the buffer do not align with header_length and body_length. I.e. if the header data you write is shorter than header_length, then <space> will come to early and the body data will be within the first header_length bytes of the buffer.

When you print the buffer, it will automatically stop when reaching the zero byte:

std::cout << "whole message is: " << data_ << std::endl;

Therefore the printing in the client does not show the random data at the end of the buffer.

You then send the entire buffer to the server:

size_t request_length = sizeof(data_)/sizeof(*data_);
boost::asio::write(s, boost::asio::buffer(data_, request_length));

In the server, you read a buffer of size equal to the size of the header:

 boost::asio::async_read(socket,boost::asio::buffer(_packet.data_, _packet.header_length), ...);

So this will process the first header_length bytes of the buffer you have written in the client. As said above, the first header_length bytes of the buffer can contain the separating space and some part of the body, depending on how short the header data you wrote to the buffer in the client is. For this reason, when you print the header in the server with this line, you will see some part of the body data:

std::cout.write(_packet.data_, _packet.header_length);

You then continue to read the body by reading body_length many bytes. This will be the remaining bytes you wrote in the client. So it will contain the remaining part of the body (minus the part that was already processed as part of the header), the zero byte and then the random data.

You print the buffer with this line, which does not stop at the zero-byte:

std::cout.write(_packet.data_, _packet.body_length);

Therefore, you will see the cut-off part of the body and then some random data in your log.

How to solve this?

First, when you create your buffer, you should make sure it does not contain random data by zero-initializing it. Otherwise you have the risk that sensitive data (cryptographic keys, passwords) may be leaked when sending the buffer over the network. Consider the following example for how to zero-initialize:

  std::cout << "Printing test" << std::endl;

  // Not zero-initialized, will print garbage.
  char test[10];
  std::cout.write(test, sizeof(test));
  std::cout << std::endl;

  std::cout << "Printing test2" << std::endl;

  // Zero-initialized, will print nothing (because \0 is not printable)
  char test2[10]{};
  std::cout.write(test2, sizeof(test2));
  std::cout << std::endl;

Second, you should make sure that you insert padding between the header data and the body data in case the header data is shorter than header_length. There are of course many different ways for doing this. Here is one example:

  int op_code = 1;
  std::string header = std::to_string(op_code);
  constexpr size_t header_length = 8;

  std::string body{"abc"};
  constexpr size_t body_length = 32;

  // Zero-initialize buffer, requires #include <array>
  std::array<char, header_length + body_length> buffer{};

  // TODO Check header.size() <= header_length
  // TODO Check body.size() <= body_length

  std::copy(header.begin(), header.end(), buffer.begin());
  std::copy(body.begin(), body.end(), buffer.begin() + header_length);

You can inspect the buffer content with:

  // Debug: Print buffer content, requires #include <iomanip>
  for (char c : buffer) {
    std::cout << std::hex << std::setw(2) << std::setfill('0') << static_cast<int>(c) << ' ';
  }
  std::cout << std::endl;

Which would give you:

31 00 00 00 00 00 00 00 61 62 63 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

You can see no random data appears, because the buffer has been zero initialized. The header data is only of size 1 byte, but padding bytes are present between the end of the header data and the body data starting after header_length bytes.