From be6af511bd6cf1d57fc56bca136119e1240bbde7 Mon Sep 17 00:00:00 2001 From: izzy Date: Sat, 7 Mar 2026 15:00:07 -0500 Subject: [PATCH 01/15] feat: cleanup exceptions a bit --- Minecraft.Client/PlayerConnection.cpp | 15 +++++++++++++++ Minecraft.World/Exceptions.h | 21 +++++++++++---------- Minecraft.World/Packet.cpp | 23 ++++++++--------------- 3 files changed, 34 insertions(+), 25 deletions(-) diff --git a/Minecraft.Client/PlayerConnection.cpp b/Minecraft.Client/PlayerConnection.cpp index 9404a5d68..cd19a1ef7 100644 --- a/Minecraft.Client/PlayerConnection.cpp +++ b/Minecraft.Client/PlayerConnection.cpp @@ -35,6 +35,9 @@ #include "..\Minecraft.World\net.minecraft.world.item.crafting.h" #include "Options.h" +// MinecraftConsoles Added +#include "..\Minecraft.World\Exceptions.h" + Random PlayerConnection::random; PlayerConnection::PlayerConnection(MinecraftServer *server, Connection *connection, shared_ptr player) @@ -88,6 +91,18 @@ void PlayerConnection::tick() didTick = false; tickCount++; + + try + { + connection->tick(); + } + catch (const IOException& e) + { + app.DebugPrintF("IOException -- %ls\n", e.information); + disconnect(DisconnectPacket::eDisconnect_None); + return; + } + connection->tick(); if(done) return; diff --git a/Minecraft.World/Exceptions.h b/Minecraft.World/Exceptions.h index 6f3ba75e6..df113ce90 100644 --- a/Minecraft.World/Exceptions.h +++ b/Minecraft.World/Exceptions.h @@ -1,29 +1,30 @@ #pragma once -using namespace std; -class EOFException : public std::exception +// izzint - TODO: these other exceptions should really be implemented + +class EOFException : public std::runtime_error { }; -class IllegalArgumentException : public std::exception +class IllegalArgumentException : public std::runtime_error { public: - wstring information; + std::wstring information; - IllegalArgumentException(const wstring& information); + IllegalArgumentException(const std::wstring& information); }; -class IOException : public std::exception +class IOException : public std::runtime_error { public: - wstring information; + std::wstring information; - IOException(const wstring& information); + IOException(const std::wstring& information); }; -class RuntimeException : public std::exception +class RuntimeException : public std::runtime_error { public: - RuntimeException(const wstring& information); + RuntimeException(const std::wstring& information); }; \ No newline at end of file diff --git a/Minecraft.World/Packet.cpp b/Minecraft.World/Packet.cpp index 61e3cc63c..0a080f0cf 100644 --- a/Minecraft.World/Packet.cpp +++ b/Minecraft.World/Packet.cpp @@ -157,9 +157,9 @@ IllegalArgumentException::IllegalArgumentException(const wstring& information) this->information = information; } -IOException::IOException(const wstring& information) +IOException::IOException(const wstring &info) : std::runtime_error("IOException") { - this->information = information; + this->information = info; } Packet::Packet() : createTime( System::currentTimeMillis() ) @@ -379,12 +379,10 @@ void Packet::writePacket(shared_ptr packet, DataOutputStream *dos) // th void Packet::writeUtf(const wstring& value, DataOutputStream *dos) // throws IOException TODO 4J JEV, should this declare a throws? { -#if 0 if (value.length() > Short::MAX_VALUE) { - throw new IOException(L"String too big"); + throw IOException(L"Packet::writeUtf - String too big"); } -#endif dos->writeShort((short)value.length()); dos->writeChars(value); @@ -394,20 +392,15 @@ wstring Packet::readUtf(DataInputStream *dis, int maxLength) // throws IOExcepti { short stringLength = dis->readShort(); - if (stringLength > maxLength) + + if (stringLength > maxLength || stringLength < 0) { - wstringstream stream; - stream << L"Received string length longer than maximum allowed (" << stringLength << " > " << maxLength << ")"; - assert(false); - // throw new IOException( stream.str() ); - } - if (stringLength < 0) - { - assert(false); - // throw new IOException(L"Received string length is less than zero! Weird string!"); + throw IOException(L"Packet::readUtf - Invalid string passed"); } wstring builder = L""; + builder.reserve(stringLength); + for (int i = 0; i < stringLength; i++) { wchar_t rc = dis->readChar(); From cfce17c19bc688c73faf173799ab9b293ab2e136 Mon Sep 17 00:00:00 2001 From: izzy Date: Sat, 7 Mar 2026 17:05:15 -0500 Subject: [PATCH 02/15] feat: implement all exception types --- Minecraft.World/Exceptions.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Minecraft.World/Exceptions.h b/Minecraft.World/Exceptions.h index df113ce90..8aabb94a3 100644 --- a/Minecraft.World/Exceptions.h +++ b/Minecraft.World/Exceptions.h @@ -4,7 +4,10 @@ class EOFException : public std::runtime_error { +public: + std::wstring information; + EOFException(const std::wstring &information); }; class IllegalArgumentException : public std::runtime_error @@ -26,5 +29,7 @@ public: class RuntimeException : public std::runtime_error { public: + std::wstring information; + RuntimeException(const std::wstring& information); }; \ No newline at end of file From e0408115eab9d1516fb215b47283c88d070c6424 Mon Sep 17 00:00:00 2001 From: izzy Date: Sat, 7 Mar 2026 17:06:19 -0500 Subject: [PATCH 03/15] feat: improve handling in `Packet` --- Minecraft.World/Exceptions.h | 2 -- Minecraft.World/Packet.cpp | 32 ++++++++++++-------------------- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/Minecraft.World/Exceptions.h b/Minecraft.World/Exceptions.h index 8aabb94a3..2a167732c 100644 --- a/Minecraft.World/Exceptions.h +++ b/Minecraft.World/Exceptions.h @@ -1,7 +1,5 @@ #pragma once -// izzint - TODO: these other exceptions should really be implemented - class EOFException : public std::runtime_error { public: diff --git a/Minecraft.World/Packet.cpp b/Minecraft.World/Packet.cpp index 0a080f0cf..96efd2e63 100644 --- a/Minecraft.World/Packet.cpp +++ b/Minecraft.World/Packet.cpp @@ -152,15 +152,13 @@ void Packet::staticCtor() map(255, true, true, true, false, typeid(DisconnectPacket), DisconnectPacket::create); } -IllegalArgumentException::IllegalArgumentException(const wstring& information) -{ - this->information = information; -} +IllegalArgumentException::IllegalArgumentException(const wstring &info) : std::runtime_error("IllegalArgumentException"), information(info) {} -IOException::IOException(const wstring &info) : std::runtime_error("IOException") -{ - this->information = info; -} +EOFException::EOFException(const wstring &info) : std::runtime_error("EOFException"), information(info) {} + +IOException::IOException(const wstring &info) : std::runtime_error("IOException"), information(info) {} + +RuntimeException::RuntimeException(const wstring &info) : std::runtime_error("RuntimeException"), information(info) {} Packet::Packet() : createTime( System::currentTimeMillis() ) { @@ -282,12 +280,7 @@ byteArray Packet::readBytes(DataInputStream *datainputstream) int size = datainputstream->readShort(); if (size < 0) { - app.DebugPrintf("Key was smaller than nothing! Weird key!"); -#ifndef _CONTENT_PACKAGE - __debugbreak(); -#endif - return byteArray(); - //throw new IOException("Key was smaller than nothing! Weird key!"); + throw IOException(L"Key was smaller than nothing! Weird key!"); } byteArray bytes(size); @@ -296,7 +289,6 @@ byteArray Packet::readBytes(DataInputStream *datainputstream) return bytes; } - bool Packet::canSendToAnyClient(shared_ptr packet) { int packetId = packet->getId(); @@ -329,14 +321,14 @@ shared_ptr Packet::readPacket(DataInputStream *dis, bool isServer) // th if ((isServer && serverReceivedPackets.find(id) == serverReceivedPackets.end()) || (!isServer && clientReceivedPackets.find(id) == clientReceivedPackets.end())) { - //app.DebugPrintf("Bad packet id %d\n", id); - __debugbreak(); - assert(false); - // throw new IOException(wstring(L"Bad packet id ") + std::to_wstring(id)); + throw IOException(wstring(L"Bad packet id ") + std::to_wstring(id)); } packet = getPacket(id); - if (packet == NULL) assert(false);//throw new IOException(wstring(L"Bad packet id ") + std::to_wstring(id)); + if (packet == nullptr) + { + throw IOException(wstring(L"Bad packet id ") + std::to_wstring(id)); + } //app.DebugPrintf("%s reading packet %d\n", isServer ? "Server" : "Client", packet->getId()); packet->read(dis); From 839ce24b9ee85da1b1660d54b38cb2aa84819592 Mon Sep 17 00:00:00 2001 From: izzy Date: Sat, 7 Mar 2026 17:06:44 -0500 Subject: [PATCH 04/15] feat: implement all serverside exceptions --- Minecraft.Client/PlayerConnection.cpp | 36 ++++++++++++++++++++------- Minecraft.World/DataInputStream.cpp | 13 +++++++--- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/Minecraft.Client/PlayerConnection.cpp b/Minecraft.Client/PlayerConnection.cpp index cd19a1ef7..548fcd885 100644 --- a/Minecraft.Client/PlayerConnection.cpp +++ b/Minecraft.Client/PlayerConnection.cpp @@ -81,12 +81,13 @@ PlayerConnection::~PlayerConnection() void PlayerConnection::tick() { - if( done ) return; - - if( m_bCloseOnTick ) - { - disconnect( DisconnectPacket::eDisconnect_Closed ); - return; + if (done || m_bCloseOnTick) + { + if (m_bCloseOnTick && !done) + { + disconnect(DisconnectPacket::eDisconnect_Closed); + } + return; } didTick = false; @@ -98,12 +99,29 @@ void PlayerConnection::tick() } catch (const IOException& e) { - app.DebugPrintF("IOException -- %ls\n", e.information); - disconnect(DisconnectPacket::eDisconnect_None); + app.DebugPrintf("IOException - %ls\n", e.information.c_str()); + disconnect(DisconnectPacket::eDisconnect_UnexpectedPacket); return; } + catch (const RuntimeException &e) + { + app.DebugPrintf("RuntimeException - %ls\n", e.information.c_str()); + disconnect(DisconnectPacket::eDisconnect_None); // izzint - no good disconnect packet for this, just keep as-is + return; + } + catch (const IllegalArgumentException &e) + { + app.DebugPrintf("IllegalArgumentException - %ls\n", e.information.c_str()); + disconnect(DisconnectPacket::eDisconnect_None); // izzint - no good disconnect packet for this, just keep as-is + return; + } + catch (const EOFException &e) + { + app.DebugPrintf("EOFException - %ls\n", e.information.c_str()); + disconnect(DisconnectPacket::eDisconnect_EndOfStream); + return; + } - connection->tick(); if(done) return; if ((tickCount - lastKeepAliveTick) > 20 * 1) diff --git a/Minecraft.World/DataInputStream.cpp b/Minecraft.World/DataInputStream.cpp index 18deed397..5dd3172bb 100644 --- a/Minecraft.World/DataInputStream.cpp +++ b/Minecraft.World/DataInputStream.cpp @@ -16,7 +16,14 @@ DataInputStream::DataInputStream(InputStream *in) : stream( in ) //This method simply performs in.read() and returns the result. int DataInputStream::read() { - return stream->read(); + int result = stream->read(); + + if (result == -1) + { + throw EOFException(L"DataInputStream::read - end of stream"); // izzint - implementation 4j didn't do for some reason + } + + return result; } //Reads some number of bytes from the contained input stream and stores them into the buffer array b. @@ -134,7 +141,7 @@ bool DataInputStream::readFully(byteArray b) int byteRead = stream->read(); if( byteRead == -1 ) { - return false; + throw EOFException(L"DataInputStream::readFully - end of stream"); } else { @@ -153,7 +160,7 @@ bool DataInputStream::readFully(charArray b) int byteRead = stream->read(); if( byteRead == -1 ) { - return false; + throw EOFException(L"DataInputStream::readFully - end of stream"); } else { From 0a8b0cb4068483129e26ff058c3d667e49904ed5 Mon Sep 17 00:00:00 2001 From: izzy Date: Sat, 7 Mar 2026 17:40:14 -0500 Subject: [PATCH 05/15] fix: impl shutdown error handling --- Minecraft.Client/ClientConnection.cpp | 29 +++++++++++++++-- Minecraft.World/Connection.cpp | 46 ++++++++++++++------------- 2 files changed, 51 insertions(+), 24 deletions(-) diff --git a/Minecraft.Client/ClientConnection.cpp b/Minecraft.Client/ClientConnection.cpp index 9b955cae9..572774fac 100644 --- a/Minecraft.Client/ClientConnection.cpp +++ b/Minecraft.Client/ClientConnection.cpp @@ -151,8 +151,33 @@ ClientConnection::~ClientConnection() void ClientConnection::tick() { - if (!done) connection->tick(); - connection->flush(); + if (done) return; + + try + { + connection->tick(); + connection->flush(); + } + catch (const IOException &e) + { + app.DebugPrintf("IOException - %ls\n", e.information.c_str()); + this->done; + } + catch (const RuntimeException &e) + { + app.DebugPrintf("RuntimeException - %ls\n", e.information.c_str()); + this->done; + } + catch (const IllegalArgumentException &e) + { + app.DebugPrintf("IllegalArgumentException - %ls\n", e.information.c_str()); + this->done; + } + catch (const EOFException &e) + { + app.DebugPrintf("EOFException - %ls\n", e.information.c_str()); + this->done; + } } INetworkPlayer *ClientConnection::getNetworkPlayer() diff --git a/Minecraft.World/Connection.cpp b/Minecraft.World/Connection.cpp index 09f72be09..d6e7fdfc7 100644 --- a/Minecraft.World/Connection.cpp +++ b/Minecraft.World/Connection.cpp @@ -583,30 +583,32 @@ int Connection::runRead(void* lpParam) con->readThreads++; LeaveCriticalSection(cs); - //try { + try { + MemSect(19); + while (con->running && !con->quitting && ShutdownManager::ShouldRun(ShutdownManager::eConnectionReadThreads)) + { + while (con->readTick()) + ; - MemSect(19); - while (con->running && !con->quitting && ShutdownManager::ShouldRun(ShutdownManager::eConnectionReadThreads)) - { - while (con->readTick()) - ; - - // try { - //Sleep(100L); - // TODO - 4J Stu - 1.8.2 changes these sleeps to 2L, but not sure whether we should do that as well - con->m_hWakeReadThread->WaitForSignal(100L); + // try { + //Sleep(100L); + // TODO - 4J Stu - 1.8.2 changes these sleeps to 2L, but not sure whether we should do that as well + con->m_hWakeReadThread->WaitForSignal(100L); + } + MemSect(0); } - MemSect(0); - - /* 4J JEV, removed try/catch - } catch (InterruptedException e) { - } - } - } finally { - synchronized (threadCounterLock) { - readThreads--; - } - } */ + catch (EOFException e) + { + app.DebugPrintf("EOFException - %ls\n", e.information.c_str()); + con->running = false; + con->quitting = true; + } + catch (IOException e) + { + app.DebugPrintf("IOException - %ls\n", e.information.c_str()); + con->running = false; + con->quitting = true; + } ShutdownManager::HasFinished(ShutdownManager::eConnectionReadThreads); return 0; From d31479a0ab18c6dd0ba90d0e1ca645640d07005f Mon Sep 17 00:00:00 2001 From: izzy Date: Sat, 7 Mar 2026 18:10:32 -0500 Subject: [PATCH 06/15] feat: implement bad chat packet handling --- Minecraft.Client/ClientConnection.cpp | 11 ++++++++++- Minecraft.World/ChatPacket.cpp | 15 ++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/Minecraft.Client/ClientConnection.cpp b/Minecraft.Client/ClientConnection.cpp index 572774fac..76507ad9c 100644 --- a/Minecraft.Client/ClientConnection.cpp +++ b/Minecraft.Client/ClientConnection.cpp @@ -2921,7 +2921,16 @@ void ClientConnection::handleContainerOpen(shared_ptr packe case ContainerOpenPacket::LARGE_CHEST: chestString = IDS_CHEST_LARGE; break; case ContainerOpenPacket::ENDER_CHEST: chestString = IDS_TILE_ENDERCHEST; break; case ContainerOpenPacket::CONTAINER: chestString = IDS_CHEST; break; - default: assert(false); chestString = -1; break; + default: + throw IOException(L"ClientConnection::handleContainerOpen - invalid container type"); + } + + + + // izzint - surprised how little checks are here + const int MAX_CONTAINER_SIZE = 54; // double chest should be max, right? + if (packet->size < 0 || packet->size > MAX_CONTAINER_SIZE) { + throw IOException(L"ClientConnection::handleContainerOpen - invalid container size"); } if( player->openContainer(shared_ptr( new SimpleContainer(chestString, packet->title, packet->customName, packet->size) ))) diff --git a/Minecraft.World/ChatPacket.cpp b/Minecraft.World/ChatPacket.cpp index 2988962e7..7bcfa9a90 100644 --- a/Minecraft.World/ChatPacket.cpp +++ b/Minecraft.World/ChatPacket.cpp @@ -43,12 +43,25 @@ ChatPacket::ChatPacket(const wstring& message, EChatPacketMessage type, int sour // Read chat packet (throws IOException) void ChatPacket::read(DataInputStream *dis) { - m_messageType = (EChatPacketMessage) dis->readShort(); + // izzint - TODO: i could see this validation being hardcoded becoming + // a problem for people who want to expand on chat types, plz review! + short msgType = dis->readShort(); + if (msgType < e_ChatCustom || msgType > e_ChatCommandTeleportToMe) + { + throw IOException(L"ChatPacket::read - invalid chat type"); + } + m_messageType = static_cast(msgType); short packedCounts = dis->readShort(); int stringCount = (packedCounts >> 4) & 0xF; int intCount = (packedCounts >> 0) & 0xF; + // izzint - again, why didn't 4j patch this out?? + if (stringCount > 15 || intCount > MAX_LENGTH) + { + throw IOException(L"ChatPacket::read - too many string arguments"); + } + for(int i = 0; i < stringCount; i++) { m_stringArgs.push_back(readUtf(dis, MAX_LENGTH)); From 76e1328b6b02c2233b3b72e9f7c7fe3108ef63ee Mon Sep 17 00:00:00 2001 From: izzy Date: Sat, 7 Mar 2026 18:23:01 -0500 Subject: [PATCH 07/15] fix: tighten chatpacket limits to bare mininum --- Minecraft.World/ChatPacket.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Minecraft.World/ChatPacket.cpp b/Minecraft.World/ChatPacket.cpp index 7bcfa9a90..bb191aace 100644 --- a/Minecraft.World/ChatPacket.cpp +++ b/Minecraft.World/ChatPacket.cpp @@ -57,7 +57,7 @@ void ChatPacket::read(DataInputStream *dis) int intCount = (packedCounts >> 0) & 0xF; // izzint - again, why didn't 4j patch this out?? - if (stringCount > 15 || intCount > MAX_LENGTH) + if (stringCount > 3 || intCount > 1) { throw IOException(L"ChatPacket::read - too many string arguments"); } From 126d08a6f8afe8045a8c69fcd0eb38d2f5795462 Mon Sep 17 00:00:00 2001 From: x86 <126331+x86@users.noreply.github.com> Date: Sun, 8 Mar 2026 13:02:36 +1300 Subject: [PATCH 08/15] RCE Fix #1 --- Minecraft.World/compression.cpp | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/Minecraft.World/compression.cpp b/Minecraft.World/compression.cpp index 99c5228ab..7413f7f35 100644 --- a/Minecraft.World/compression.cpp +++ b/Minecraft.World/compression.cpp @@ -196,9 +196,20 @@ HRESULT Compression::DecompressLZXRLE(void *pDestination, unsigned int *pDestSiz unsigned int rleSize = staticRleSize; unsigned char *dynamicRleBuf = NULL; - if(*pDestSize > rleSize) + + unsigned int safeRleSize = max(rleSize, *pDestSize); + + const unsigned int MAX_RLE_ALLOC = 16 * 1024 * 1024; // 16 MB + if(safeRleSize > MAX_RLE_ALLOC) { - rleSize = *pDestSize; + LeaveCriticalSection(&rleDecompressLock); + *pDestSize = 0; + return E_FAIL; + } + + if(safeRleSize > staticRleSize) + { + rleSize = safeRleSize; dynamicRleBuf = new unsigned char[rleSize]; Decompress(dynamicRleBuf, &rleSize, pSource, SrcSize); pucIn = (unsigned char *)dynamicRleBuf; @@ -212,16 +223,19 @@ HRESULT Compression::DecompressLZXRLE(void *pDestination, unsigned int *pDestSiz //unsigned char *pucIn = (unsigned char *)rleDecompressBuf; unsigned char *pucEnd = pucIn + rleSize; unsigned char *pucOut = (unsigned char *)pDestination; + unsigned char *pucOutEnd = pucOut + *pDestSize; while( pucIn != pucEnd ) { unsigned char thisOne = *pucIn++; if( thisOne == 255 ) { + if( pucIn >= pucEnd ) break; unsigned int count = *pucIn++; if( count < 3 ) { count++; + if( pucOut + count > pucOutEnd ) { pucOut = pucOutEnd; break; } for( unsigned int i = 0; i < count; i++ ) { *pucOut++ = 255; @@ -230,7 +244,9 @@ HRESULT Compression::DecompressLZXRLE(void *pDestination, unsigned int *pDestSiz else { count++; + if( pucIn >= pucEnd ) break; unsigned char data = *pucIn++; + if( pucOut + count > pucOutEnd ) { pucOut = pucOutEnd; break; } for( unsigned int i = 0; i < count; i++ ) { *pucOut++ = data; @@ -239,6 +255,7 @@ HRESULT Compression::DecompressLZXRLE(void *pDestination, unsigned int *pDestSiz } else { + if( pucOut >= pucOutEnd ) break; *pucOut++ = thisOne; } } @@ -260,16 +277,19 @@ HRESULT Compression::DecompressRLE(void *pDestination, unsigned int *pDestSize, unsigned char *pucIn = (unsigned char *)pSource; unsigned char *pucEnd = pucIn + SrcSize; unsigned char *pucOut = (unsigned char *)pDestination; + unsigned char *pucOutEnd = pucOut + *pDestSize; while( pucIn != pucEnd ) { unsigned char thisOne = *pucIn++; if( thisOne == 255 ) { + if( pucIn >= pucEnd ) break; unsigned int count = *pucIn++; if( count < 3 ) { count++; + if( pucOut + count > pucOutEnd ) { pucOut = pucOutEnd; break; } for( unsigned int i = 0; i < count; i++ ) { *pucOut++ = 255; @@ -278,7 +298,9 @@ HRESULT Compression::DecompressRLE(void *pDestination, unsigned int *pDestSize, else { count++; + if( pucIn >= pucEnd ) break; unsigned char data = *pucIn++; + if( pucOut + count > pucOutEnd ) { pucOut = pucOutEnd; break; } for( unsigned int i = 0; i < count; i++ ) { *pucOut++ = data; @@ -287,6 +309,7 @@ HRESULT Compression::DecompressRLE(void *pDestination, unsigned int *pDestSize, } else { + if( pucOut >= pucOutEnd ) break; *pucOut++ = thisOne; } } @@ -542,5 +565,3 @@ void Compression::SetDecompressionType(ESavePlatform platform) } /*Compression gCompression;*/ - - From 5d63f83286dd232f5a3f2b084dd4c9e997664c81 Mon Sep 17 00:00:00 2001 From: x86 <126331+x86@users.noreply.github.com> Date: Sun, 8 Mar 2026 13:08:28 +1300 Subject: [PATCH 09/15] Rce Fix 2 --- Minecraft.World/TextureAndGeometryPacket.cpp | 23 ++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/Minecraft.World/TextureAndGeometryPacket.cpp b/Minecraft.World/TextureAndGeometryPacket.cpp index d28fc8628..8cbe8d6d7 100644 --- a/Minecraft.World/TextureAndGeometryPacket.cpp +++ b/Minecraft.World/TextureAndGeometryPacket.cpp @@ -121,7 +121,17 @@ void TextureAndGeometryPacket::read(DataInputStream *dis) //throws IOException { textureName = dis->readUTF(); dwSkinID = (DWORD)dis->readInt(); - dwTextureBytes = (DWORD)dis->readShort(); + + short rawTextureBytes = dis->readShort(); + if(rawTextureBytes <= 0) + { + dwTextureBytes = 0; + } + else + { + dwTextureBytes = (DWORD)(unsigned short)rawTextureBytes; + if(dwTextureBytes > 65536) dwTextureBytes = 0; + } if(dwTextureBytes>0) { @@ -134,7 +144,16 @@ void TextureAndGeometryPacket::read(DataInputStream *dis) //throws IOException } uiAnimOverrideBitmask = dis->readInt(); - dwBoxC = (DWORD)dis->readShort(); + short rawBoxC = dis->readShort(); + if(rawBoxC <= 0) + { + dwBoxC = 0; + } + else + { + dwBoxC = (DWORD)(unsigned short)rawBoxC; + if(dwBoxC > 256) dwBoxC = 0; // sane limit for skin boxes + } if(dwBoxC>0) { From 7e3b59305300a878bf1d676c4d749fa3050130df Mon Sep 17 00:00:00 2001 From: x86 <126331+x86@users.noreply.github.com> Date: Sun, 8 Mar 2026 13:10:17 +1300 Subject: [PATCH 10/15] Rce Fix 3 --- Minecraft.World/TexturePacket.cpp | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/Minecraft.World/TexturePacket.cpp b/Minecraft.World/TexturePacket.cpp index 77dfdc38c..a057c7173 100644 --- a/Minecraft.World/TexturePacket.cpp +++ b/Minecraft.World/TexturePacket.cpp @@ -37,16 +37,24 @@ void TexturePacket::handle(PacketListener *listener) void TexturePacket::read(DataInputStream *dis) //throws IOException { textureName = dis->readUTF(); - dwBytes = (DWORD)dis->readShort(); - - if(dwBytes>0) + short rawBytes = dis->readShort(); + if(rawBytes <= 0) { - this->pbData= new BYTE [dwBytes]; + dwBytes = 0; + return; + } + dwBytes = (DWORD)(unsigned short)rawBytes; + if(dwBytes > 65536) + { + dwBytes = 0; + return; + } + + this->pbData= new BYTE [dwBytes]; - for(DWORD i=0;ipbData[i] = dis->readByte(); - } + for(DWORD i=0;ipbData[i] = dis->readByte(); } } From 17cf9ba34d6f3d137b901a8b3244753c747f852c Mon Sep 17 00:00:00 2001 From: izzy Date: Sat, 7 Mar 2026 20:09:35 -0500 Subject: [PATCH 11/15] feat: patch out readutf, limit still needed. --- Minecraft.World/DataInputStream.cpp | 49 ++++++++++++++++------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/Minecraft.World/DataInputStream.cpp b/Minecraft.World/DataInputStream.cpp index 5dd3172bb..12029fbf3 100644 --- a/Minecraft.World/DataInputStream.cpp +++ b/Minecraft.World/DataInputStream.cpp @@ -305,10 +305,20 @@ unsigned short DataInputStream::readUnsignedShort() //a Unicode string. wstring DataInputStream::readUTF() { - wstring outputString; int a = stream->read(); int b = stream->read(); + if (a == -1 || b == -1) + { + throw EOFException(L"DataInputStream::readUTF - end of stream"); + } + unsigned short UTFLength = (unsigned short) (((a & 0xff) << 8) | (b & 0xff)); + const unsigned short UTF_MAX_LENGTH = 32767; + + if (UTFLength > UTF_MAX_LENGTH) + { + throw IOException(L"DataInputStream::readUTF - string exceeds maximum length"); + } //// 4J Stu - I decided while writing DataOutputStream that we didn't need to bother using the UTF8 format //// used in the java libs, and just write in/out as wchar_t all the time @@ -319,6 +329,9 @@ wstring DataInputStream::readUTF() outputString.push_back(theChar); }*/ + // izzint - let's hope our checks before work! :] + wstring outputString; + outputString.reserve(UTFLength); unsigned short currentByteIndex = 0; while( currentByteIndex < UTFLength ) @@ -326,9 +339,10 @@ wstring DataInputStream::readUTF() int firstByte = stream->read(); currentByteIndex++; - if( firstByte == -1 ) - // TODO 4J Stu - EOFException - break; + if (firstByte == -1) + { + throw EOFException(L"DataInputStream::readUTF - end of stream"); + } // Masking patterns: // 10000000 = 0x80 // Match only highest bit @@ -344,8 +358,7 @@ wstring DataInputStream::readUTF() // 1110xxxx = 0xE0 // Three byte UTF if( ( (firstByte & 0xC0 ) == 0x80 ) || ( (firstByte & 0xF0) == 0xF0) ) { - // TODO 4J Stu - UTFDataFormatException - break; + throw IOException(L"DataInputStream::readUTF - invalid UTF-8 byte"); } else if( (firstByte & 0x80) == 0x00 ) { @@ -361,8 +374,7 @@ wstring DataInputStream::readUTF() // No more bytes to read if( !(currentByteIndex < UTFLength) ) { - // TODO 4J Stu - UTFDataFormatException - break; + throw IOException(L"DataInputStream::readUTF - invalid UTF character"); } int secondByte = stream->read(); @@ -371,14 +383,12 @@ wstring DataInputStream::readUTF() // No second byte if( secondByte == -1 ) { - // TODO 4J Stu - EOFException - break; + throw EOFException(L"DataInputStream::readUTF - end of stream"); } // Incorrect second byte pattern else if( (secondByte & 0xC0 ) != 0x80 ) { - // TODO 4J Stu - UTFDataFormatException - break; + throw IOException(L"DataInputStream::readUTF - invalid UTF character"); } wchar_t readChar = (wchar_t)( ((firstByte& 0x1F) << 6) | (secondByte & 0x3F) ); @@ -392,8 +402,7 @@ wstring DataInputStream::readUTF() // No more bytes to read if( !(currentByteIndex < UTFLength) ) { - // TODO 4J Stu - UTFDataFormatException - break; + throw IOException(L"DataInputStream::readUTF - invalid UTF character"); } int secondByte = stream->read(); @@ -402,15 +411,13 @@ wstring DataInputStream::readUTF() // No second byte if( secondByte == -1 ) { - // TODO 4J Stu - EOFException - break; + throw EOFException(L"DataInputStream::readUTF - end of stream"); } // No more bytes to read if( !(currentByteIndex < UTFLength) ) { - // TODO 4J Stu - UTFDataFormatException - break; + throw IOException(L"DataInputStream::readUTF - invalid UTF character"); } int thirdByte = stream->read(); @@ -419,14 +426,12 @@ wstring DataInputStream::readUTF() // No third byte if( thirdByte == -1 ) { - // TODO 4J Stu - EOFException - break; + throw EOFException(L"DataInputStream::readUTF - end of stream"); } // Incorrect second or third byte pattern else if( ( (secondByte & 0xC0 ) != 0x80 ) || ( (thirdByte & 0xC0 ) != 0x80 ) ) { - // TODO 4J Stu - UTFDataFormatException - break; + throw IOException(L"DataInputStream::readUTF - invalid UTF character"); } wchar_t readChar = (wchar_t)(((firstByte & 0x0F) << 12) | ((secondByte & 0x3F) << 6) | (thirdByte & 0x3F)); From dacde328c9e2668dfef4954f499b89e0564a4af2 Mon Sep 17 00:00:00 2001 From: x86 <126331+x86@users.noreply.github.com> Date: Sun, 8 Mar 2026 14:47:59 +1300 Subject: [PATCH 12/15] RCE Fix 4 --- Minecraft.World/BlockRegionUpdatePacket.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Minecraft.World/BlockRegionUpdatePacket.cpp b/Minecraft.World/BlockRegionUpdatePacket.cpp index bec943d8d..afac43685 100644 --- a/Minecraft.World/BlockRegionUpdatePacket.cpp +++ b/Minecraft.World/BlockRegionUpdatePacket.cpp @@ -103,6 +103,12 @@ void BlockRegionUpdatePacket::read(DataInputStream *dis) //throws IOException levelIdx = ( size >> 30 ) & 3; size &= 0x3fffffff; + const int MAX_COMPRESSED_CHUNK_SIZE = 5 * 1024 * 1024; + if(size < 0 || size > MAX_COMPRESSED_CHUNK_SIZE) + { + size = 0; + } + if(size == 0) { buffer = byteArray(); @@ -131,7 +137,10 @@ void BlockRegionUpdatePacket::read(DataInputStream *dis) //throws IOException delete [] compressedBuffer.data; - assert(buffer.length == outputSize); + if(buffer.length != outputSize) + { + app.DebugPrintf("BlockRegionUpdatePacket: decompressed size mismatch (expected %d, got %d)\n", buffer.length, outputSize); + } } } From 99ab1e64645e183e227efcbf40a326bb5ec02fd2 Mon Sep 17 00:00:00 2001 From: izzy Date: Sat, 7 Mar 2026 21:12:42 -0500 Subject: [PATCH 13/15] feat: fully patch datastream --- Minecraft.World/DataInputStream.cpp | 135 +++++++++++++++++----------- 1 file changed, 84 insertions(+), 51 deletions(-) diff --git a/Minecraft.World/DataInputStream.cpp b/Minecraft.World/DataInputStream.cpp index 12029fbf3..a24f533be 100644 --- a/Minecraft.World/DataInputStream.cpp +++ b/Minecraft.World/DataInputStream.cpp @@ -90,7 +90,15 @@ void DataInputStream::close() //the boolean value read. bool DataInputStream::readBoolean() { - return stream->read() != 0; + int val = stream->read(); + + // izzint - strange situation + if ((val) == -1) + { + throw EOFException(L"DataInputStream::readBoolean - end of stream"); + } + + return val != 0; } //Reads and returns one input byte. The byte is treated as a signed value in the range -128 through 127, inclusive. @@ -99,12 +107,28 @@ bool DataInputStream::readBoolean() //the 8-bit value read. byte DataInputStream::readByte() { - return (byte) stream->read(); + int val = stream->read(); + + // izzint - strange situation + if ((val) == -1) + { + throw EOFException(L"DataInputStream::readByte - end of stream"); + } + + return static_cast(val); } unsigned char DataInputStream::readUnsignedByte() { - return (unsigned char) stream->read(); + int val = stream->read(); + + // izzint - strange situation + if ((val) == -1) + { + throw EOFException(L"DataInputStream::readUnsignedByte - end of stream"); + } + + return static_cast(val); } //Reads two input bytes and returns a char value. Let a be the first byte read and b be the second byte. The value returned is: @@ -117,6 +141,13 @@ wchar_t DataInputStream::readChar() { int a = stream->read(); int b = stream->read(); + + // izzint - strange situation + if ((a | b) < 0) + { + throw EOFException(L"DataInputStream::readChar - end of stream"); + } + return (wchar_t)((a << 8) | (b & 0xff)); } @@ -151,25 +182,6 @@ bool DataInputStream::readFully(byteArray b) return true; } -bool DataInputStream::readFully(charArray b) -{ - // TODO 4J Stu - I am not entirely sure if this matches the implementation of the Java library - // TODO 4J Stu - Need to handle exceptions here is we throw them in other InputStreams - for(unsigned int i = 0; i < b.length ;i++) - { - int byteRead = stream->read(); - if( byteRead == -1 ) - { - throw EOFException(L"DataInputStream::readFully - end of stream"); - } - else - { - b[i] = byteRead; - } - } - return true; -} - //Reads eight input bytes and returns a double value. It does this by first constructing a long value in exactly the manner //of the readlong method, then converting this long value to a double in exactly the manner of the method Double.longBitsToDouble. //This method is suitable for reading bytes written by the writeDouble method of interface DataOutput. @@ -208,6 +220,13 @@ int DataInputStream::readInt() int b = stream->read(); int c = stream->read(); int d = stream->read(); + + // izzint - strange situation + if ((a | b | c | d) < 0) + { + throw EOFException(L"DataInputStream::readInt - end of stream"); + } + int bits = (((a & 0xff) << 24) | ((b & 0xff) << 16) | ((c & 0xff) << 8) | (d & 0xff)); return bits; @@ -239,14 +258,20 @@ int64_t DataInputStream::readLong() int64_t g = stream->read(); int64_t h = stream->read(); - int64_t bits = (((a & 0xff) << 56) | - ((b & 0xff) << 48) | - ((c & 0xff) << 40) | - ((d & 0xff) << 32) | - ((e & 0xff) << 24) | - ((f & 0xff) << 16) | - ((g & 0xff) << 8) | - ((h & 0xff))); + // izzint - strange situation + if ((a | b | c | d | e| f | g | h) < 0) + { + throw EOFException(L"DataInputStream::readLong - end of stream"); + } + + int64_t bits = ((((int64_t)(a & 0xff)) << 56) | + (((int64_t)(b & 0xff)) << 48) | + (((int64_t)(c & 0xff)) << 40) | + (((int64_t)(d & 0xff)) << 32) | + (((int64_t)(e & 0xff)) << 24) | + (((int64_t)(f & 0xff)) << 16) | + (((int64_t)(g & 0xff)) << 8) | + ((int64_t)(h & 0xff))); return bits; } @@ -261,6 +286,13 @@ short DataInputStream::readShort() { int a = stream->read(); int b = stream->read(); + + // izzint - strange situation + if ((a | b) < 0) + { + throw EOFException(L"DataInputStream::readShort - end of stream"); + } + return (short)((a << 8) | (b & 0xff)); } @@ -268,6 +300,13 @@ unsigned short DataInputStream::readUnsignedShort() { int a = stream->read(); int b = stream->read(); + + // izzint - strange situation + if ((a | b) < 0) + { + throw EOFException(L"DataInputStream::readUnsignedShort - end of stream"); + } + return (unsigned short)((a << 8) | (b & 0xff)); } @@ -331,7 +370,6 @@ wstring DataInputStream::readUTF() // izzint - let's hope our checks before work! :] wstring outputString; - outputString.reserve(UTFLength); unsigned short currentByteIndex = 0; while( currentByteIndex < UTFLength ) @@ -445,12 +483,12 @@ wstring DataInputStream::readUTF() int DataInputStream::readUTFChar() { - int returnValue = -1; int firstByte = stream->read(); - if( firstByte == -1 ) - // TODO 4J Stu - EOFException - return returnValue; + if (firstByte == -1) + { + throw EOFException(L"DataInputStream::readUTFChar - end of stream"); + } // Masking patterns: // 10000000 = 0x80 // Match only highest bit @@ -466,13 +504,12 @@ int DataInputStream::readUTFChar() // 1110xxxx = 0xE0 // Three byte UTF if( ( (firstByte & 0xC0 ) == 0x80 ) || ( (firstByte & 0xF0) == 0xF0) ) { - // TODO 4J Stu - UTFDataFormatException - return returnValue; + throw IOException(L"DataInputStream::readUTFChar - invalid UTF character"); } else if( (firstByte & 0x80) == 0x00 ) { // One byte UTF - returnValue = firstByte; + return firstByte; } else if( (firstByte & 0xE0) == 0xC0 ) { @@ -482,17 +519,15 @@ int DataInputStream::readUTFChar() // No second byte if( secondByte == -1 ) { - // TODO 4J Stu - EOFException - return returnValue; + throw EOFException(L"DataInputStream::readUTFChar - end of stream"); } // Incorrect second byte pattern else if( (secondByte & 0xC0 ) != 0x80 ) { - // TODO 4J Stu - UTFDataFormatException - return returnValue; + throw IOException(L"DataInputStream::readUTFChar - invalid UTF character"); } - returnValue = ((firstByte& 0x1F) << 6) | (secondByte & 0x3F); + return ((firstByte& 0x1F) << 6) | (secondByte & 0x3F); } else if( (firstByte & 0xF0) == 0xE0 ) { @@ -503,8 +538,7 @@ int DataInputStream::readUTFChar() // No second byte if( secondByte == -1 ) { - // TODO 4J Stu - EOFException - return returnValue; + throw EOFException(L"DataInputStream::readUTFChar - end of stream"); } int thirdByte = stream->read(); @@ -512,19 +546,18 @@ int DataInputStream::readUTFChar() // No third byte if( thirdByte == -1 ) { - // TODO 4J Stu - EOFException - return returnValue; + throw EOFException(L"DataInputStream::readUTFChar - end of stream"); } // Incorrect second or third byte pattern else if( ( (secondByte & 0xC0 ) != 0x80 ) || ( (thirdByte & 0xC0 ) != 0x80 ) ) { - // TODO 4J Stu - UTFDataFormatException - return returnValue; + throw IOException(L"DataInputStream::readUTFChar - invalid UTF character"); } - returnValue = (((firstByte & 0x0F) << 12) | ((secondByte & 0x3F) << 6) | (thirdByte & 0x3F)); + return (((firstByte & 0x0F) << 12) | ((secondByte & 0x3F) << 6) | (thirdByte & 0x3F)); } - return returnValue; + + throw IOException(L"DataInputStream::readUTFChar - unknown byte pattern"); // izzint - somebody who knows more about utf-8 should make these better } // 4J Added From 7dc6009fc09261df10ae7e357e696041e8d1da13 Mon Sep 17 00:00:00 2001 From: izzy Date: Sat, 7 Mar 2026 21:28:09 -0500 Subject: [PATCH 14/15] fix: restore charArray version of `readFully` --- Minecraft.World/DataInputStream.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Minecraft.World/DataInputStream.cpp b/Minecraft.World/DataInputStream.cpp index a24f533be..444e50443 100644 --- a/Minecraft.World/DataInputStream.cpp +++ b/Minecraft.World/DataInputStream.cpp @@ -182,6 +182,20 @@ bool DataInputStream::readFully(byteArray b) return true; } +bool DataInputStream::readFully(charArray b) +{ + for (unsigned int i = 0; i < b.length; i++) + { + int byteRead = stream->read(); + if (byteRead == -1) + { + throw EOFException(L"DataInputStream::readFully - end of stream"); + } + b[i] = byteRead; + } + return true; +} + //Reads eight input bytes and returns a double value. It does this by first constructing a long value in exactly the manner //of the readlong method, then converting this long value to a double in exactly the manner of the method Double.longBitsToDouble. //This method is suitable for reading bytes written by the writeDouble method of interface DataOutput. From 82bb77a3a53a8bae539d46bdb0a145d5d887b814 Mon Sep 17 00:00:00 2001 From: izzy Date: Sun, 8 Mar 2026 11:56:12 -0400 Subject: [PATCH 15/15] fix: cleanup, start replacing error handling --- Minecraft.Client/PlayerConnection.cpp | 3 --- Minecraft.World/DataInputStream.cpp | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/Minecraft.Client/PlayerConnection.cpp b/Minecraft.Client/PlayerConnection.cpp index 548fcd885..9dee3abc2 100644 --- a/Minecraft.Client/PlayerConnection.cpp +++ b/Minecraft.Client/PlayerConnection.cpp @@ -35,9 +35,6 @@ #include "..\Minecraft.World\net.minecraft.world.item.crafting.h" #include "Options.h" -// MinecraftConsoles Added -#include "..\Minecraft.World\Exceptions.h" - Random PlayerConnection::random; PlayerConnection::PlayerConnection(MinecraftServer *server, Connection *connection, shared_ptr player) diff --git a/Minecraft.World/DataInputStream.cpp b/Minecraft.World/DataInputStream.cpp index 444e50443..4dc628f01 100644 --- a/Minecraft.World/DataInputStream.cpp +++ b/Minecraft.World/DataInputStream.cpp @@ -501,7 +501,7 @@ int DataInputStream::readUTFChar() if (firstByte == -1) { - throw EOFException(L"DataInputStream::readUTFChar - end of stream"); + return -1; } // Masking patterns: