[Server Crash Fix] Packet sending and buffer reallocation

[Distraught 10074]

Hello,

Today I found a new possible crash again.

So, in DESC::Packet - if _IMPROVED_PACKET_ENCRYPTION_ is enabled -, we store a pointer the the underlying buffer's write point of m_lpOutputBuffer.
Then later in the packet_encode function, we might reallocate this buffer if its size is not enough to write our new packet there. 
After this, we give the originally stored pointer to Encrypt function to encrypt the packet data. If the buffer was reallocated, it's a dangling pointer.

 

How to fix this?
In desc.cpp go to DESC::Packet function.
Store the write position of that buffer instead of the memory address.
Then, when calling the Encrypt function, pass it a pointer to the underlying buffer of m_lpOutputBuffer, offset by the stored write position.

So, it should look something like this:

int write_point_pos = m_lpOutputBuffer->write_point_pos;
if (packet_encode(m_lpOutputBuffer, c_pvData, iSize))
{
	void* buf = m_lpOutputBuffer->mem_data + write_point_pos;
	if (cipher_.activated()) {
		cipher_.Encrypt(buf, iSize);
	}
}

 

Additional Note

If you haven't already fixed your __packet_encode function, then make the input LPBUFFER a reference, so it will actually reallocate your buffer and you won't have a dangling pointer again.

Before:

inline bool __packet_encode(LPBUFFER pbuf, const void * data, int length, const char * file, int line)

After:

inline bool __packet_encode(LPBUFFER& pbuf, const void * data, int length, const char * file, int line)

 

[Helia01 3416]

thx, but In my opinion, _IMPROVED_PACKET_ENCRYPTION_ is a poorly implemented feature, because when a server is attacked, for example, phantom packet header errors occur.

Of course, it's important to understand that such harmful traffic shouldn't reach game cores, but it can still be a problem simply when there are a large number of players in a city.
My opinion is that this feature should be disabled or a protobuf and ChaCha20 should be used.

Edited September 23, 2025 by Helia01

[Mitachi 3392]

9 hours ago, Helia01 said:

In my opinion, _IMPROVED_PACKET_ENCRYPTION_ is a poorly implemented feature, because when a server is attacked, for example, phantom packet header errors occur.

Of course, it's important to understand that such harmful traffic shouldn't reach game cores, but it can still be a problem simply when there are a large number of players in a city.
My opinion is that this feature should be disabled or a protobuf and ChaCha20 should be used.

1. This fix is very useful; I'm probably the only file seller who keeps IPE disabled, which means that out of 100 servers, at least 90 (counting those who don't disable it) will have it enabled. Therefore, fixing it/making it stabler is a good move.
    
2. protobuf have nothing to do with data encryption, it is a serialization system

Edited September 23, 2025 by Mitachi
Better not to flame

[Luigina 33]

I agree to @ Mitachi If you are not turning off IPE, I recommend that you do so. If you are going to turn off IPE, I recommend that you also turn off xtea encryption. This way, you can use it without any problems without editing any code. If you are going to actively use xtea encryption, there are a few things you need to adjust. I cannot explain these because some of my friends are selling them.

[Metin2 Dev 12287]

Good job 

If I remember corectly the last part with packet_encode is also a memory leak fix 

[Lead0b110010100 28]

9 hours ago, Mitachi said:

1. This fix is very useful; I'm probably the only file seller who keeps IPE disabled, which means that out of 100 servers, at least 90 (counting those who don't disable it) will have it enabled. Therefore, fixing it/making it stabler is a good move.
    
2. protobuf have nothing to do with data encryption, it is a serialization system

Others speak english and this man speaks in facts. This way of communicating feels like reading a bitch slap.

GJ!