search

lsalzman / enet

ENet reliable UDP networking library
MIT License
2.78k stars 675 forks source link

Undefined behaviour in fragment calculations #256

Closed arvid-norlander closed 5 months ago

arvid-norlander commented 5 months ago

Using UBSAN I ran into this (in a proprietary application unfortunately):

enet/protocol.c:627:63: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
    #0 0x79e84e6469a9 in enet_protocol_handle_send_fragment (enet/protocol.c:627 (discriminator 1))
    #1 0x79e84e64ce98 in enet_protocol_handle_incoming_commands (enet/protocol.c:1171)
    #2 0x79e84e64d6e8 in enet_protocol_receive_incoming_commands (enet/protocol.c:1282)
    #3 0x79e84e654d70 in enet_host_service (enet/protocol.c:1846)
    [... propietary code ...]

enet/protocol.c:631:62: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
    #0 0x79e84e646b50 in enet_protocol_handle_send_fragment (enet/protocol.c:631 (discriminator 1))
    #1 0x79e84e64ce98 in enet_protocol_handle_incoming_commands (enet/protocol.c:1171)
    #2 0x79e84e64d6e8 in enet_protocol_receive_incoming_commands (enet/protocol.c:1282)
    #3 0x79e84e654d70 in enet_host_service (enet/protocol.c:1846)
    [... propietary code ...]

This is using Enet 1.3.18.

Analysis: From what I can tell, the fragmentNumber % 32 becomes 31, and it is UB to do that left shift in signed integers. I will submitting a PR on this shortly.