facebookarchive / RakNet

RakNet is a cross platform, open source, C++ networking engine for game programmers.
Other
3.29k stars 1.02k forks source link

splitPacketCount vulnerability #143

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.