Validate netplay packet fields (#1189)

The netplay server controls packet lengths, player numbers, event counts,
and controller pak values. The client used those fields without bounds
checks, allowing out-of-bounds accesses and unbounded event allocation.

Validate packet records and player and pak values, cap pending events,
and handle allocation failure.
This commit is contained in:
Acts1631
2026-10-02 21:02:13 +02:00
committed by GitHub
parent c5dfc44764
commit ba95bab92a
+49 -7
View File
@@ -20,6 +20,7 @@
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
#define SETTINGS_SIZE 24
#define NETPLAY_MAX_PENDING_EVENTS 255
#define M64P_CORE_PROTOTYPES 1
#include "api/callbacks.h"
@@ -231,6 +232,24 @@ static uint8_t buffer_size(uint8_t control_id)
return counter;
}
static int netplay_packet_has_data(const UDPpacket* packet, size_t offset, size_t size)
{
return packet->len >= 0 && offset <= (size_t) packet->len
&& size <= (size_t) packet->len - offset;
}
static uint8_t netplay_validate_plugin(uint8_t player, uint8_t plugin)
{
if ((player > 0 && plugin == PLUGIN_MEMPAK)
|| (plugin != PLUGIN_NONE && plugin != PLUGIN_MEMPAK
&& plugin != PLUGIN_RUMBLE_PAK && plugin != PLUGIN_RAW))
{
return PLUGIN_NONE;
}
return plugin;
}
static void netplay_request_input(uint8_t control_id)
{
l_request_input_packet->data[0] = UDP_REQUEST_KEY_INFO;
@@ -284,11 +303,28 @@ static void netplay_process()
uint8_t plugin, player, current_status;
while (SDLNet_UDP_Recv(l_udpSocket, l_process_packet) == 1)
{
if (!netplay_packet_has_data(l_process_packet, 0, 5))
{
DebugMessage(M64MSG_ERROR, "Netplay: received truncated message from server");
continue;
}
switch (l_process_packet->data[0])
{
case UDP_RECEIVE_KEY_INFO:
case UDP_RECEIVE_KEY_INFO_GRATUITOUS:
player = l_process_packet->data[1];
if (player >= 4)
{
DebugMessage(M64MSG_ERROR, "Netplay: received invalid player from server");
break;
}
if (l_process_packet->data[4] >
((size_t) l_process_packet->len - 5) / 9)
{
DebugMessage(M64MSG_ERROR, "Netplay: received truncated input data from server");
break;
}
//current_status is a status update from the server
//it will let us know if another player has disconnected, or the games have desynced
current_status = l_process_packet->data[2];
@@ -319,13 +355,24 @@ static void netplay_process()
continue;
}
if (buffer_size(player) >= NETPLAY_MAX_PENDING_EVENTS)
{
DebugMessage(M64MSG_ERROR, "Netplay: received too many pending input events");
break;
}
keys = SDLNet_Read32(&l_process_packet->data[curr]);
curr += 4;
plugin = l_process_packet->data[curr];
plugin = netplay_validate_plugin(player, l_process_packet->data[curr]);
curr += 1;
//insert new event at beginning of linked list
struct netplay_event* new_event = (struct netplay_event*)malloc(sizeof(struct netplay_event));
if (new_event == NULL)
{
DebugMessage(M64MSG_ERROR, "Netplay: could not allocate input event");
break;
}
new_event->count = count;
new_event->buttons = keys;
new_event->plugin = plugin;
@@ -628,12 +675,7 @@ void netplay_read_registration(struct controller_input_compat* cin_compats)
else
{
Controls[i].Present = 1;
if (i > 0 && input_data[curr] == PLUGIN_MEMPAK) // only P1 can use mempak
Controls[i].Plugin = PLUGIN_NONE;
else if (input_data[curr] == PLUGIN_TRANSFER_PAK) // Transferpak not supported during netplay
Controls[i].Plugin = PLUGIN_NONE;
else
Controls[i].Plugin = input_data[curr];
Controls[i].Plugin = netplay_validate_plugin(i, input_data[curr]);
l_plugin[i] = Controls[i].Plugin;
++curr;
Controls[i].RawData = input_data[curr];