P25 receive loop can be permanently stopped by zero-length or undersized UDP packets

[ki6uve]

Summary

The P25 reflector does not validate UDP packet length before indexing into the received buffer. A zero-length UDP datagram causes an exception at:

byte opcode = buffer[0];

Because the exception handler surrounds the entire while loop in ReceiveLoop(), the exception is logged and the P25 receive task exits permanently instead of discarding the malformed packet and continuing.

This appears to create a remote denial-of-service / availability issue for any publicly reachable P25 reflector.

Affected code

P25_Reflector/P25Reflector.cs

private async Task ReceiveLoop(CancellationToken token)
{
    try
    {
        while (!token.IsCancellationRequested)
        {
            var (buffer, senderAddress) = _networkManager.ReceiveData();
            if (buffer != null)
            {
                HandleIncomingData(buffer, senderAddress);
            }
            await Task.Delay(10, token);
        }
    }
    catch (Exception ex)
    {
        _logger.Error(ex, "P25 Reflector died!! error in recieve loop");
    }
}

HandleIncomingData() immediately accesses buffer[0] without confirming that the datagram contains at least one byte.

There are additional packet-length assumptions later in the handler:

• NET_POLL constructs a P25Peer, which reads 10 callsign bytes beginning at offset 1. It therefore requires at least 11 bytes.
• Opcode 0x64 reads buffer[1] and requires at least 2 bytes.
• Opcodes 0x65 and 0x66 read through buffer[3] and require at least 4 bytes.

Impact

A malformed UDP datagram can terminate the P25 receive loop.

The larger application process may remain alive because Program.Main() does not observe or restart the terminated P25 receive task. In a Docker deployment, this can leave the container appearing healthy while the P25 reflector is no longer processing traffic.

Expected behavior

Malformed, empty, or undersized UDP datagrams should be rejected and logged at a suitable level without terminating the P25 receive loop.

Suggested fix

1. Validate the packet length before reading any buffer index.
2. Validate opcode-specific minimum lengths before processing packet contents.
3. Handle malformed-packet exceptions inside the loop so one bad datagram cannot stop the listener.
4. Consider rate-limited warning logging to prevent malformed-packet log flooding.

For example:

private async Task ReceiveLoop(CancellationToken token)
{
    while (!token.IsCancellationRequested)
    {
        try
        {
            var (buffer, senderAddress) = _networkManager.ReceiveData();

            if (buffer == null || buffer.Length == 0)
                continue;

            HandleIncomingData(buffer, senderAddress);
        }
        catch (Exception ex)
        {
            _logger.Warning(ex, "P25: Dropped malformed UDP packet");
        }

        await Task.Delay(10, token);
    }
}

Then add opcode-specific checks in HandleIncomingData(), such as:

if (buffer.Length < 1)
    return;

byte opcode = buffer[0];

switch (opcode)
{
    case Opcode.NET_POLL:
        if (buffer.Length < 11)
            return;
        break;

    case 0x64:
        if (buffer.Length < 2)
            return;
        break;

    case 0x65:
    case 0x66:
        if (buffer.Length < 4)
            return;
        break;
}

Severity suggestion

I would classify this as an unauthenticated remote denial-of-service / availability issue for Internet-exposed P25 reflector instances. I have not identified a code-execution path; the concern is that a malformed UDP packet can silently disable the P25 receive service until it is restarted.

[firealarmss]

So if you just wanted to give me an AI made issue, why didn't you just have AI resolve it?

[ki6uve]

Oh, yeah...I should have tried that. I will try that tomorrow to see if it will come up with a fix.