DoS by Abusing Replay Protection

[RandyGaul]

See the replay protection where it is called in netcode_read_packet here: https://github.com/networkprotocol/netcode.io/blob/master/netcode.c#L1874-L1883

int netcode_replay_protection_packet_already_received( struct netcode_replay_protection_t * replay_protection, uint64_t sequence )
{
    netcode_assert( replay_protection );

    if ( sequence + NETCODE_REPLAY_PROTECTION_BUFFER_SIZE <= replay_protection->most_recent_sequence )
        return 1;

    if ( sequence > replay_protection->most_recent_sequence )
        replay_protection->most_recent_sequence = sequence;

    int index = (int) ( sequence % NETCODE_REPLAY_PROTECTION_BUFFER_SIZE );

    if ( replay_protection->received_packet[index] == 0xFFFFFFFFFFFFFFFFLL )
    {
        replay_protection->received_packet[index] = sequence;
        return 0;
    }

    if ( replay_protection->received_packet[index] >= sequence )
        return 1;

    replay_protection->received_packet[index] = sequence;

    return 0;
}

Replay protection happens before:

1. Decryption
2. Checking IP address from recvfrom

All an attacker would have to do to DoS another client is send the client a packet with a large sequence number. The connection will be halted, as the replay protection will cull all valid sequence numbers, since they are "too small" to fit into the buffer due to this check:

if ( sequence + NETCODE_REPLAY_PROTECTION_BUFFER_SIZE <= replay_protection->most_recent_sequence )

This is also a problem for the server, and is not safe against IP spoofing -- anyone can spoof a valid client's IP address and send a large sequence number, especially in an innocuous keepalive packet.

Solution: Separate the replay protection into two stages. One stage that checks for duplicates before decryption (as an optimization), and the other stage to track the maximum sequence number after decryption succeeds (thus validating the sequence number, since a client valid client will presumably never DoS themselves by sending an artificially large sequence, since that makes no sense).

Here's a test case confirming the problem: https://gist.github.com/RandyGaul/cabfd7441bb89f5c0a38e6f5fb152f60

Here's a pull-request fixing the problem: https://github.com/networkprotocol/netcode.io/pull/85

[RandyGaul]

I have no idea how one would actually get a hold of another player's IP address, but if they somehow do then this DoS attack would be a problem.

[RandyGaul]

Oh I think the standard will also need to be updated. It’s really late for me right now, but I can add that to my PR tomorrow.

[gafferongames]

There is no vulnerability here. A malicious client cannot send a packet with an arbitrary sequence number, since they don't have the private key for the communication. The sequence number would fail the signature check.

[RandyGaul]

They don’t need a key. Replay protection happens before decryption. See the test I provided. The code writes the max sequence after reading the sequence number from a packet before the encryption check fails.

The test simply sends a packet to the server with no key. The packet is almost entirely randomized bits, and fails the encryption test, and still triggers the attack.

The pull request you just closed is using the decryption check before tracking the max sequence.

[pushrax]

This looks like a legitimate issue, can you revisit #85 @gafferongames?