SLikeSoft / SLikeNet

SLikeNet™ is an Open Source/Free Software cross-platform network engine written in C++ and specifially designed for games (and applications which have comparable requirements on a network engine like games) building upon the discontinued RakNet network engine which had more than 13 years of active development.
https://www.slikenet.com/
Other
395 stars 62 forks source link

splitPacketCount vulnerability #60

Open C0lumbo opened 4 years ago

C0lumbo commented 4 years ago

A corrupt packet with a high value for splitPacketCount can cause either a crash (bad_array_new_length exception) or an infinite loop in the call to newChannel->splitPacketList.Preallocate.

Such a corrupted packet could arise by chance or be crafted by a malicious user to cause a server or peer to crash. After running into the issue in the wild, I found that the issue was briefly raised on the RakNet forums here but no fix was deployed: http://www.raknet.com/forum/index.php?topic=5253.msg21668#msg21668

My local fix was to change this block in ReliabilityLayer::CreateInternalPacketFromBitStream from:

    if (readSuccess==false ||
        internalPacket->dataBitLength==0 ||
        internalPacket->reliability>=NUMBER_OF_RELIABILITIES ||
        internalPacket->orderingChannel>=32 || 
        (hasSplitPacket && (internalPacket->splitPacketIndex >= internalPacket->splitPacketCount)))
    {
        // If this assert hits, encoding is garbage
        RakAssert("Encoding is garbage" && 0);
        ReleaseToInternalPacketPool( internalPacket );
        return 0;
    }

to

    const int maxPacketSize = 1024 * 1024 * 4;
    const int maxPacketSplit = (maxPacketSize + (MINIMUM_MTU_SIZE - 1)) / MINIMUM_MTU_SIZE;
    if (readSuccess==false ||
        internalPacket->dataBitLength==0 ||
        internalPacket->reliability>=NUMBER_OF_RELIABILITIES ||
        internalPacket->orderingChannel>=32 || 
        internalPacket->splitPacketCount > maxPacketSplit ||
        (hasSplitPacket && (internalPacket->splitPacketIndex >= internalPacket->splitPacketCount)))
    {
        // If this assert hits, encoding is garbage
        RakAssert("Encoding is garbage" && 0);
        ReleaseToInternalPacketPool( internalPacket );
        return 0;
    }

However, my fix limits packet sends to 4 megabytes, which is more than enough for my projects, but is not suitable for RakNet as a whole.

Perhaps a better fix is to define the maximum packet size in RakNetDefines.h to allow users to easily override. The maximum should also be enforced in RakPeer::Send and similar functions.

Issue also posted to original RakNet repository, but I think action is more likely here: https://github.com/facebookarchive/RakNet/issues/143