From 2498318e6e163ef7a092b8a0d5102789f335ae1e Mon Sep 17 00:00:00 2001 From: JuelzIrons Date: Mon, 9 Mar 2026 23:40:18 -0400 Subject: [PATCH 1/3] Fix 3 RCEs in packet parsing and texture loading --- Minecraft.Client/AbstractTexturePack.cpp | 5 +-- .../Network/DQRNetworkManager_SendReceive.cpp | 42 ++++++++++++++++--- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/Minecraft.Client/AbstractTexturePack.cpp b/Minecraft.Client/AbstractTexturePack.cpp index a3c677272..cd6c497df 100644 --- a/Minecraft.Client/AbstractTexturePack.cpp +++ b/Minecraft.Client/AbstractTexturePack.cpp @@ -296,8 +296,7 @@ void AbstractTexturePack::loadDefaultHTMLColourTable() const DWORD LOCATOR_SIZE = 256; // Use this to allocate space to hold a ResourceLocator string WCHAR szResourceLocator[ LOCATOR_SIZE ]; - // Try and load the HTMLColours.col based off the common XML first, before the deprecated xuiscene_colourtable - wsprintfW(szResourceLocator,L"section://%X,%s#%s",c_ModuleHandle,L"media", L"media/HTMLColours.col"); + swprintf_s(szResourceLocator, LOCATOR_SIZE, L"section://%X,%s#%s",c_ModuleHandle,L"media", L"media/HTMLColours.col"); BYTE *data; UINT dataLength; if(XuiResourceLoadAll(szResourceLocator, &data, &dataLength) == S_OK) @@ -308,7 +307,7 @@ void AbstractTexturePack::loadDefaultHTMLColourTable() } else { - wsprintfW(szResourceLocator,L"section://%X,%s#%s",c_ModuleHandle,L"media", L"media/"); + swprintf_s(szResourceLocator, LOCATOR_SIZE, L"section://%X,%s#%s",c_ModuleHandle,L"media", L"media/"); HXUIOBJ hScene; HRESULT hr = XuiSceneCreate(szResourceLocator,L"xuiscene_colourtable.xur", nullptr, &hScene); diff --git a/Minecraft.Client/Durango/Network/DQRNetworkManager_SendReceive.cpp b/Minecraft.Client/Durango/Network/DQRNetworkManager_SendReceive.cpp index eed3e8511..b8fbe8748 100644 --- a/Minecraft.Client/Durango/Network/DQRNetworkManager_SendReceive.cpp +++ b/Minecraft.Client/Durango/Network/DQRNetworkManager_SendReceive.cpp @@ -209,15 +209,35 @@ void DQRNetworkManager::BytesReceivedInternal(DQRConnectionInfo *connectionInfo, break; case DQRConnectionInfo::ConnectionState_InternalRoomSyncData: connectionInfo->m_pucRoomSyncData[connectionInfo->m_roomSyncDataBytesRead++] = byte; - // The room sync info is sent as a 4 byte count of the length of XUID strings, then the RoomSyncData, then the XUID strings if( connectionInfo->m_roomSyncDataBytesToRead == 0 ) { - // At first stage of reading the 4 byte count if( connectionInfo->m_roomSyncDataBytesRead == 4 ) { - memcpy( &connectionInfo->m_roomSyncDataBytesToRead, connectionInfo->m_pucRoomSyncData, 4); + int receivedSize = 0; + memcpy( &receivedSize, connectionInfo->m_pucRoomSyncData, 4); delete [] connectionInfo->m_pucRoomSyncData; - connectionInfo->m_roomSyncDataBytesToRead += sizeof(RoomSyncData); + + if( receivedSize < 0 || receivedSize > 100000 ) + { + connectionInfo->m_internalDataState = DQRConnectionInfo::ConnectionState_InternalHeaderByte; + connectionInfo->m_pucRoomSyncData = nullptr; + connectionInfo->m_roomSyncDataBytesToRead = 0; + connectionInfo->m_roomSyncDataBytesRead = 0; + break; + } + + int totalSize = receivedSize + sizeof(RoomSyncData); + + if( totalSize < receivedSize ) + { + connectionInfo->m_internalDataState = DQRConnectionInfo::ConnectionState_InternalHeaderByte; + connectionInfo->m_pucRoomSyncData = nullptr; + connectionInfo->m_roomSyncDataBytesToRead = 0; + connectionInfo->m_roomSyncDataBytesRead = 0; + break; + } + + connectionInfo->m_roomSyncDataBytesToRead = totalSize; connectionInfo->m_pucRoomSyncData = new unsigned char[ connectionInfo->m_roomSyncDataBytesToRead ]; connectionInfo->m_roomSyncDataBytesRead = 0; } @@ -296,8 +316,20 @@ void DQRNetworkManager::BytesReceivedInternal(DQRConnectionInfo *connectionInfo, // At first stage of reading the 4 byte count if( connectionInfo->m_addFailedPlayerDataBytesRead == 4 ) { - memcpy( &connectionInfo->m_addFailedPlayerDataBytesToRead, connectionInfo->m_pucAddFailedPlayerData, 4); + int receivedSize = 0; + memcpy( &receivedSize, connectionInfo->m_pucAddFailedPlayerData, 4); delete [] connectionInfo->m_pucAddFailedPlayerData; + + if( receivedSize < 0 || receivedSize > 100000 ) + { + connectionInfo->m_internalDataState = DQRConnectionInfo::ConnectionState_InternalHeaderByte; + connectionInfo->m_pucAddFailedPlayerData = nullptr; + connectionInfo->m_addFailedPlayerDataBytesToRead = 0; + connectionInfo->m_addFailedPlayerDataBytesRead = 0; + break; + } + + connectionInfo->m_addFailedPlayerDataBytesToRead = receivedSize; connectionInfo->m_pucAddFailedPlayerData = new unsigned char[ connectionInfo->m_addFailedPlayerDataBytesToRead ]; connectionInfo->m_addFailedPlayerDataBytesRead = 0; } From 1cfad1a88c7c5db4bb6a01a0d5892c952a06a4be Mon Sep 17 00:00:00 2001 From: JuelzIrons Date: Tue, 10 Mar 2026 00:10:53 -0400 Subject: [PATCH 2/3] Logging + size increase to prevent disconnect --- .../Durango/Network/DQRNetworkManager_SendReceive.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Minecraft.Client/Durango/Network/DQRNetworkManager_SendReceive.cpp b/Minecraft.Client/Durango/Network/DQRNetworkManager_SendReceive.cpp index b8fbe8748..d3b070536 100644 --- a/Minecraft.Client/Durango/Network/DQRNetworkManager_SendReceive.cpp +++ b/Minecraft.Client/Durango/Network/DQRNetworkManager_SendReceive.cpp @@ -217,8 +217,9 @@ void DQRNetworkManager::BytesReceivedInternal(DQRConnectionInfo *connectionInfo, memcpy( &receivedSize, connectionInfo->m_pucRoomSyncData, 4); delete [] connectionInfo->m_pucRoomSyncData; - if( receivedSize < 0 || receivedSize > 100000 ) + if( receivedSize < 0 || receivedSize > 1048576 ) { + DQRNetworkManager::LogCommentFormat(L"RoomSyncData rejected: size=%d", receivedSize); connectionInfo->m_internalDataState = DQRConnectionInfo::ConnectionState_InternalHeaderByte; connectionInfo->m_pucRoomSyncData = nullptr; connectionInfo->m_roomSyncDataBytesToRead = 0; @@ -320,8 +321,9 @@ void DQRNetworkManager::BytesReceivedInternal(DQRConnectionInfo *connectionInfo, memcpy( &receivedSize, connectionInfo->m_pucAddFailedPlayerData, 4); delete [] connectionInfo->m_pucAddFailedPlayerData; - if( receivedSize < 0 || receivedSize > 100000 ) + if( receivedSize < 0 || receivedSize > 1048576 ) { + DQRNetworkManager::LogCommentFormat(L"AddFailedPlayerData rejected: size=%d", receivedSize); connectionInfo->m_internalDataState = DQRConnectionInfo::ConnectionState_InternalHeaderByte; connectionInfo->m_pucAddFailedPlayerData = nullptr; connectionInfo->m_addFailedPlayerDataBytesToRead = 0; From c50e06f564509cf5402212a6edbe38c1240938fb Mon Sep 17 00:00:00 2001 From: JuelzIrons Date: Tue, 10 Mar 2026 00:37:28 -0400 Subject: [PATCH 3/3] info about size limits not sure if this is what you meant by 'could you document where this maximum comes from in the code' but i assume you meant this --- .../Durango/Network/DQRNetworkManager_SendReceive.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Minecraft.Client/Durango/Network/DQRNetworkManager_SendReceive.cpp b/Minecraft.Client/Durango/Network/DQRNetworkManager_SendReceive.cpp index d3b070536..b02ec449f 100644 --- a/Minecraft.Client/Durango/Network/DQRNetworkManager_SendReceive.cpp +++ b/Minecraft.Client/Durango/Network/DQRNetworkManager_SendReceive.cpp @@ -217,7 +217,8 @@ void DQRNetworkManager::BytesReceivedInternal(DQRConnectionInfo *connectionInfo, memcpy( &receivedSize, connectionInfo->m_pucRoomSyncData, 4); delete [] connectionInfo->m_pucRoomSyncData; - if( receivedSize < 0 || receivedSize > 1048576 ) + // a 512KB limit gives enough headroom for legitimate XUID data, but prevents a overflow. With 2 players the size is about 100kb so 512kb gives good headroom + if( receivedSize < 0 || receivedSize > 524288 ) { DQRNetworkManager::LogCommentFormat(L"RoomSyncData rejected: size=%d", receivedSize); connectionInfo->m_internalDataState = DQRConnectionInfo::ConnectionState_InternalHeaderByte; @@ -321,7 +322,8 @@ void DQRNetworkManager::BytesReceivedInternal(DQRConnectionInfo *connectionInfo, memcpy( &receivedSize, connectionInfo->m_pucAddFailedPlayerData, 4); delete [] connectionInfo->m_pucAddFailedPlayerData; - if( receivedSize < 0 || receivedSize > 1048576 ) + // a 512KB limit gives enough headroom for legitimate XUID strings, but prevents a overflow. + if( receivedSize < 0 || receivedSize > 524288 ) { DQRNetworkManager::LogCommentFormat(L"AddFailedPlayerData rejected: size=%d", receivedSize); connectionInfo->m_internalDataState = DQRConnectionInfo::ConnectionState_InternalHeaderByte;