diff --git a/Minecraft.Client/ClientConnection.cpp b/Minecraft.Client/ClientConnection.cpp index 0aa9735d8..a06a16685 100644 --- a/Minecraft.Client/ClientConnection.cpp +++ b/Minecraft.Client/ClientConnection.cpp @@ -158,8 +158,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() @@ -2941,7 +2966,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.Client/PlayerConnection.cpp b/Minecraft.Client/PlayerConnection.cpp index 9404a5d68..9dee3abc2 100644 --- a/Minecraft.Client/PlayerConnection.cpp +++ b/Minecraft.Client/PlayerConnection.cpp @@ -78,17 +78,47 @@ 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; tickCount++; - connection->tick(); + + try + { + connection->tick(); + } + catch (const IOException& e) + { + 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; + } + if(done) return; if ((tickCount - lastKeepAliveTick) > 20 * 1) diff --git a/Minecraft.World/BlockRegionUpdatePacket.cpp b/Minecraft.World/BlockRegionUpdatePacket.cpp index 730ce3edc..fd10a2c14 100644 --- a/Minecraft.World/BlockRegionUpdatePacket.cpp +++ b/Minecraft.World/BlockRegionUpdatePacket.cpp @@ -105,6 +105,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(); diff --git a/Minecraft.World/ChatPacket.cpp b/Minecraft.World/ChatPacket.cpp index 2988962e7..bb191aace 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 > 3 || intCount > 1) + { + throw IOException(L"ChatPacket::read - too many string arguments"); + } + for(int i = 0; i < stringCount; i++) { m_stringArgs.push_back(readUtf(dis, MAX_LENGTH)); diff --git a/Minecraft.World/Connection.cpp b/Minecraft.World/Connection.cpp index 5058baa7d..9971fcc41 100644 --- a/Minecraft.World/Connection.cpp +++ b/Minecraft.World/Connection.cpp @@ -591,30 +591,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; diff --git a/Minecraft.World/DataInputStream.cpp b/Minecraft.World/DataInputStream.cpp index 18deed397..4dc628f01 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. @@ -83,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. @@ -92,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: @@ -110,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)); } @@ -134,7 +172,7 @@ bool DataInputStream::readFully(byteArray b) int byteRead = stream->read(); if( byteRead == -1 ) { - return false; + throw EOFException(L"DataInputStream::readFully - end of stream"); } else { @@ -146,21 +184,16 @@ bool DataInputStream::readFully(byteArray b) 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 ) - { - return false; - } - else - { - b[i] = byteRead; - } - } - return true; + 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 @@ -201,6 +234,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; @@ -232,14 +272,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; } @@ -254,6 +300,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)); } @@ -261,6 +314,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)); } @@ -298,10 +358,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 @@ -312,6 +382,8 @@ wstring DataInputStream::readUTF() outputString.push_back(theChar); }*/ + // izzint - let's hope our checks before work! :] + wstring outputString; unsigned short currentByteIndex = 0; while( currentByteIndex < UTFLength ) @@ -319,9 +391,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 @@ -337,8 +410,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 ) { @@ -354,8 +426,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(); @@ -364,14 +435,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) ); @@ -385,8 +454,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(); @@ -395,15 +463,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(); @@ -412,14 +478,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)); @@ -433,12 +497,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) + { + return -1; + } // Masking patterns: // 10000000 = 0x80 // Match only highest bit @@ -454,13 +518,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 ) { @@ -470,17 +533,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 ) { @@ -491,8 +552,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(); @@ -500,19 +560,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 diff --git a/Minecraft.World/Exceptions.h b/Minecraft.World/Exceptions.h index 6f3ba75e6..2a167732c 100644 --- a/Minecraft.World/Exceptions.h +++ b/Minecraft.World/Exceptions.h @@ -1,29 +1,33 @@ #pragma once -using namespace std; -class EOFException : public std::exception -{ - -}; - -class IllegalArgumentException : public std::exception +class EOFException : public std::runtime_error { public: - wstring information; + std::wstring information; - IllegalArgumentException(const wstring& information); + EOFException(const std::wstring &information); }; -class IOException : public std::exception +class IllegalArgumentException : public std::runtime_error { public: - wstring information; + std::wstring information; - IOException(const wstring& information); + IllegalArgumentException(const std::wstring& information); }; -class RuntimeException : public std::exception +class IOException : public std::runtime_error { public: - RuntimeException(const wstring& information); + std::wstring information; + + IOException(const std::wstring& information); +}; + +class RuntimeException : public std::runtime_error +{ +public: + std::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 bc2152b2d..7244e186d 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& information) -{ - this->information = information; -} +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(); @@ -357,7 +349,10 @@ shared_ptr Packet::readPacket(DataInputStream *dis, bool isServer) // th } 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)); + } s_lastIds[s_lastIdPos] = id; s_lastIdPos = (s_lastIdPos + 1) % 8; @@ -403,12 +398,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); @@ -418,20 +411,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(); 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) { 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(); } } diff --git a/Minecraft.World/compression.cpp b/Minecraft.World/compression.cpp index b9845f010..c85816082 100644 --- a/Minecraft.World/compression.cpp +++ b/Minecraft.World/compression.cpp @@ -237,9 +237,20 @@ HRESULT Compression::DecompressLZXRLE(void *pDestination, unsigned int *pDestSiz unsigned char *dynamicRleBuf = NULL; HRESULT decompressResult; - 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]; decompressResult = Decompress(dynamicRleBuf, &rleSize, pSource, SrcSize); pucIn = (unsigned char *)dynamicRleBuf; @@ -605,5 +616,3 @@ void Compression::SetDecompressionType(ESavePlatform platform) } /*Compression gCompression;*/ - -