search

lsalzman / enet

ENet reliable UDP networking library
MIT License
2.73k stars 670 forks source link

Fix too late ENET_PEER_STATE_CONNECTED check upon enet_peer_send #158

Closed playingoDEERUX closed 3 years ago

playingoDEERUX commented 3 years ago

A potential out of bounds could occur where one misuses enet_peer_send without manually checking if peer->state was ENET_PEER_STATE_CONNECTED before, as a reference of ENetChannel in peer -> channels was attempted to be accessed after before checking if target ENetPeer is even connected. Since peer -> channels is not fixed and is dynamically allocated on connect and deallocated + set to NULL on enet_peer_reset_queues, accessing & peer -> channels [channelID] can cause an out of bounds, making the peer->state != ENET_PEER_STATE_CONNECTED check useless anywhere below.

playingoDEERUX commented 3 years ago

Honestly I am not sure if this can cause an out of bounds since it's just taking the reference of it, there may be no array access at all but I feel more confident that the check is just above every other access to peer -> channels which can be NULL.

playingoDEERUX commented 3 years ago

Nope, seems like it's fine...

But in my opinion it's still better practice to just put this entire check:

if (peer->state != ENET_PEER_STATE_CONNECTED || channelID >= peer->channelCount || packet->dataLength > peer->host->maximumPacketSize) return -1;

above everything else.

lsalzman commented 3 years ago

Fixed.