basil00 / Divert

WinDivert: Windows Packet Divert
https://reqrypt.org/windivert.html
Other
2.32k stars 491 forks source link

Infinite loop: Correct use of batched WinDivertHelperParsePacket with ppNext and pNextLen #354

Closed etkaar closed 3 months ago

etkaar commented 7 months ago

With following code I always end up with an infinite loop. Shouldn't WinDivertHelperParsePacket return false after the buffer is empty to end the loop?

UINT8 *batch_buffer;
UINT batch_buffer_len;
UINT recv_bytes_written;
UINT recv_addr_len;

PVOID payload;
UINT payload_len;

PVOID next_packet;
UINT next_packet_len;

WINDIVERT_ADDRESS *recv_addr;

char source_addr[INET_ADDRSTRLEN];
char destination_addr[INET_ADDRSTRLEN];

PWINDIVERT_IPHDR ip_header;
PWINDIVERT_UDPHDR udp_header;

batch_buffer = (UINT8 *)malloc(BATCH_SIZE * MTU);
recv_addr = (WINDIVERT_ADDRESS *)malloc(BATCH_SIZE * sizeof(WINDIVERT_ADDRESS));

if (batch_buffer == NULL || recv_addr == NULL) {
    MessageBoxError("Failed to allocate buffer (%lu). Batch size: %u.", GetLastError(), BATCH_SIZE);
    exit(EXIT_FAILURE);
}

while (true) {
    batch_buffer_len = BATCH_SIZE * MTU;
    recv_addr_len = BATCH_SIZE * sizeof(WINDIVERT_ADDRESS);

    if (!WinDivertRecvEx(config->windivert_handle, batch_buffer, batch_buffer_len, &recv_bytes_written, 0, recv_addr, &recv_addr_len, NULL)) {
        if (GetLastError() != ERROR_IO_PENDING) {
            MessageBoxError("WinDivert: Failed to read packet (%lu).", GetLastError());
            exit(EXIT_FAILURE);
        }

        continue;
    }

    while (WinDivertHelperParsePacket(batch_buffer, recv_bytes_written, &ip_header, NULL, NULL, NULL, NULL, NULL, &udp_header, &payload, &payload_len, &next_packet, &next_packet_len)) {
        if (ip_header == NULL || udp_header == NULL) {
            continue;
        }

        WinDivertHelperFormatIPv4Address(ntohl(ip_header->SrcAddr), source_addr, sizeof(source_addr));
        WinDivertHelperFormatIPv4Address(ntohl(ip_header->DstAddr), destination_addr, sizeof(destination_addr));

        printf("source_addr: %s, destination_addr: %s, next_packet_len: %u\n", source_addr, destination_addr, next_packet_len);

    }

}
majibow commented 7 months ago

My friend you have not read the documentation correctly.

By default this function will parse a single packet. However, if either ppNext or pNextLen are non-NULL, then the pPacket parameter can point to a batch (>1) of packets (and packetLen can be the total length of the batch). In this case, the function will parse the first packet, and a pointer to the remaining packet(s) will be written to ppNext, and the remaining length will be written to pNextLen. This makes it convenient to loop over every packet in the batch as follows:

while (WinDivertHelperParsePacket(pPacket, packetLen, ..., &pPacket, &packetLen))
{
    ...
}

In your code you are never updating the location of batch_buffer or recv_bytes_written therefore you always get the first packet in the buffer.

basil00 commented 3 months ago

I think majibow's answer is correct.