zpl-c / enet

⚡️ ENet reliable UDP networking library
https://blog.zpl.pw
MIT License
699 stars 60 forks source link

mismatch malloc and free in `enet_packet_create` and `enet_packet_resize` #47

Closed oylbin closed 9 months ago

oylbin commented 9 months ago

I'm using the latest enet.h from the master branch. Below is a simple test.c file to reproduce the error:

#define ENET_IMPLEMENTATION
#include "enet.h"
int main()
{
    ENetPacket *packet = enet_packet_create("packet",
                                            strlen("packet") + 1,
                                            ENET_PACKET_FLAG_RELIABLE);
    enet_packet_resize(packet, strlen("packetfoo") + 1);
}

Compile and run it, and then I encounter an error:

> gcc -I. -otest test.c && ./test
free(): invalid pointer
Aborted

Upon debugging and following the code, the error is triggered by enet_free(packet->data); in enet_packet_resize.

https://github.com/zpl-c/enet/blob/67178b030f75079b47d4c22115edc19cc10daf24/include/enet.h#L1401-L1417

The program tries to free packet->data, but it is not the correct memory address allocated by enet_malloc.

As seen in enet_packet_create, the ENetPacket packet is allocated by enet_malloc, and packet->data is an offset of this address.

https://github.com/zpl-c/enet/blob/67178b030f75079b47d4c22115edc19cc10daf24/include/enet.h#L1374-L1379

The official ENet does not have this issue as packet->data is allocated by enet_malloc.

https://github.com/lsalzman/enet/blob/2a85cd64459f6ba038d233a634d9440490dbba12/packet.c#L19-L33

I also noticed issue #44. So, I think maybe enet_packet_resize was intentionally removed initially.

Therefore, the solution may be to:

  1. Remove enet_packet_resize and clearly document to users why this library does not provide it, as it saves one malloc call in enet_packet_create.
  2. OR ensure that the malloc and free actions in enet_packet_create and enet_packet_resize match.
oylbin commented 9 months ago

enet_packet_resize was intentionally removed

https://github.com/zpl-c/enet/blob/67178b030f75079b47d4c22115edc19cc10daf24/misc/ChangeLog#L5-L6

zpl-zak commented 9 months ago

Hey, good find! I will get it fixed, _resize was re-added recently but I haven't considered we allocate the packet differently, hence the mismatch.

zpl-zak commented 9 months ago

Fixed in https://github.com/zpl-c/enet/releases/tag/v2.3.9

oylbin commented 9 months ago

hi @zpl-zak , thanks for the quick response. I think there is still an issue in the new code. The pointer passed in by the user is freed, but the pointer to the newly allocated memory is not returned out. I think the return value type of the resize function may need to be modified.

oylbin commented 9 months ago

I created a pull request

https://github.com/zpl-c/enet/pull/48

zpl-zak commented 9 months ago

Right, sorry about that, thanks for the PR!