Port over RCE Patches from LCEMP (#1023)

* LCEMP RCE Fixes WIP

Based on d017bfc30a

* Update to LCEMP's ByteArrayIO version

Fixes compilation since ours was missing some revisions from LCEMP

* Add additional safety checks missed in first pass

* Remove duplicate recipe count check

Minecraft.Client/Chunk.cpp

@@ -739,9 +739,9 @@ void Chunk::rebuild_SPU()
 				{
 					// 4J - get tile from those copied into our local array in earlier optimisation
 					unsigned char tileId = pOutData->getTile(x,y,z);
-					if (tileId > 0)
+                    if (tileId > 0 && tileId != 0xff)
 					{
-						if (currentLayer == 0 && Tile::tiles[tileId]->isEntityTile())
+                        if (currentLayer == 0 && Tile::tiles[tileId] && Tile::tiles[tileId]->isEntityTile())
 						{
 							shared_ptr<TileEntity> et = region.getTileEntity(x, y, z);
 							if (TileEntityRenderDispatcher::instance->hasRenderer(et))
@@ -754,6 +754,7 @@ void Chunk::rebuild_SPU()
 						{
 
 							Tile *tile = Tile::tiles[tileId];
+							if (!tile) continue;
 							int renderLayer = tile->getRenderLayer();
 
 							if (renderLayer != currentLayer)

Minecraft.Client/PlayerConnection.cpp

@@ -1588,13 +1588,13 @@ void PlayerConnection::handleCraftItem(shared_ptr<CraftItemPacket> packet)
 	if(iRecipe == -1)
 		return;
 
+	int recipeCount = (int)Recipes::getInstance()->getRecipies()->size();
+	if(iRecipe < 0 || iRecipe >= recipeCount)
+		return;
+
 	Recipy::INGREDIENTS_REQUIRED *pRecipeIngredientsRequired=Recipes::getInstance()->getRecipeIngredientsArray();
 	shared_ptr<ItemInstance> pTempItemInst=pRecipeIngredientsRequired[iRecipe].pRecipy->assemble(nullptr);
 
-	size_t recipeCount = Recipes::getInstance()->getRecipies()->size();
-	if (iRecipe < 0 || iRecipe >= (int)recipeCount)
-		return;
-
 	if(app.DebugSettingsOn() && (player->GetDebugOptions()&(1L<<eDebugSetting_CraftAnything)))
 	{
 		pTempItemInst->onCraftedBy(player->level, dynamic_pointer_cast<Player>( player->shared_from_this() ), pTempItemInst->count );

Minecraft.Client/ServerLevel.cpp

@@ -1077,7 +1077,12 @@ void ServerLevel::entityRemoved(shared_ptr<Entity> e)
 
 shared_ptr<Entity> ServerLevel::getEntity(int id)
 {
-	return entitiesById[id];
+    auto it = entitiesById.find(id);
+    if (it != entitiesById.end())
+    {
+        return it->second;
+    }
+    return nullptr;
 }
 
 bool ServerLevel::addGlobalEntity(shared_ptr<Entity> e)

Minecraft.Client/Windows64/Network/WinsockNetLayer.cpp

@@ -404,7 +404,7 @@ bool WinsockNetLayer::JoinGame(const char* ip, int port)
 
 bool WinsockNetLayer::SendOnSocket(SOCKET sock, const void* data, int dataSize)
 {
-	if (sock == INVALID_SOCKET || dataSize <= 0) return false;
+	if (sock == INVALID_SOCKET || dataSize <= 0 || dataSize > WIN64_NET_MAX_PACKET_SIZE) return false;
 
 	// TODO: s_sendLock is a single global lock for ALL sockets. If one client's
 	// send() blocks (TCP window full, slow WiFi), every other write thread stalls

Minecraft.World/AbstractContainerMenu.cpp

@@ -157,6 +157,9 @@ shared_ptr<ItemInstance> AbstractContainerMenu::clicked(int slotIndex, int butto
 	shared_ptr<ItemInstance> clickedEntity = nullptr;
 	shared_ptr<Inventory> inventory = player->inventory;
 
+	if (slotIndex < 0 || slotIndex >= (int)slots.size())
+		return nullptr;
+
 	if (clickType == CLICK_QUICK_CRAFT)
 	{
 		int expectedStatus = quickcraftStatus;
@@ -558,12 +561,13 @@ bool AbstractContainerMenu::isPauseScreen()
 
 void AbstractContainerMenu::setItem(unsigned int slot, shared_ptr<ItemInstance> item)
 {
+	if (slot >= slots.size()) return;
 	getSlot(slot)->set(item);
 }
 
 void AbstractContainerMenu::setAll(ItemInstanceArray *items)
 {
-	for (unsigned int i = 0; i < items->length; i++) 
+    for (unsigned int i = 0; i < items->length && i < slots.size(); i++) 
 	{
 		getSlot(i)->set( (*items)[i] );
 	}

Minecraft.World/AwardStatPacket.cpp

@@ -47,7 +47,7 @@ void AwardStatPacket::read(DataInputStream *dis) //throws IOException
 
 	// Read parameter blob.
 	int length = dis->readInt();
-	if(length > 0)
+    if (length > 0 && length <= 65536)
 	{
 		m_paramData = byteArray(length);
 		dis->readFully(m_paramData);

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();

Minecraft.World/ByteArrayInputStream.cpp

@@ -2,117 +2,141 @@
 
 #include "InputOutputStream.h"
 
-//Creates ByteArrayInputStream that uses buf as its buffer array. The initial value of pos is offset and
+// Creates ByteArrayInputStream that uses buf as its buffer array. The initial value of pos is offset and
-//the initial value of count is the minimum of offset+length and buf.length. The buffer array is not copied.
+// the initial value of count is the minimum of offset+length and buf.length. The buffer array is not copied.
-//The buffer's mark is set to the specified offset.
+// The buffer's mark is set to the specified offset.
-//Parameters:
+// Parameters:
-//buf - the input buffer.
+// buf - the input buffer.
-//offset - the offset in the buffer of the first byte to read.
+// offset - the offset in the buffer of the first byte to read.
-//length - the maximum number of bytes to read from the buffer.
+// length - the maximum number of bytes to read from the buffer.
 ByteArrayInputStream::ByteArrayInputStream(byteArray buf, unsigned int offset, unsigned int length)
-	: pos( offset ), count( min( offset+length, buf.length ) ), mark( offset )
+    : pos(offset), mark(offset)
 {
-	this->buf = buf;
+    if (offset > buf.length)
+    {
+        count = buf.length;
+    }
+    else if (length > buf.length - offset)
+    {
+        count = buf.length;
+    }
+    else
+    {
+        count = offset + length;
+    }
+    this->buf = buf;
 }
 
-//Creates a ByteArrayInputStream so that it uses buf as its buffer array. The buffer array is not copied.
+// Creates a ByteArrayInputStream so that it uses buf as its buffer array. The buffer array is not copied.
-//The initial value of pos is 0 and the initial value of count is the length of buf.
+// The initial value of pos is 0 and the initial value of count is the length of buf.
-//Parameters:
+// Parameters:
-//buf - the input buffer.
+// buf - the input buffer.
 ByteArrayInputStream::ByteArrayInputStream(byteArray buf)
-	: pos( 0 ), count( buf.length ), mark( 0 )
+    : pos(0), count(buf.length), mark(0)
 {
-	this->buf = buf;
+    this->buf = buf;
 }
 
-//Reads the next byte of data from this input stream. The value byte is returned as an int in the range 0 to 255.
+// Reads the next byte of data from this input stream. The value byte is returned as an int in the range 0 to 255.
-//If no byte is available because the end of the stream has been reached, the value -1 is returned.
+// If no byte is available because the end of the stream has been reached, the value -1 is returned.
-//This read method cannot block.
+// This read method cannot block.
-//Returns:
+// Returns:
-//the next byte of data, or -1 if the end of the stream has been reached.
+// the next byte of data, or -1 if the end of the stream has been reached.
 int ByteArrayInputStream::read()
 {
-	if( pos >= count )
+    if (pos >= count)
-		return -1;
+    {
-	else
+        return -1;
-		return buf[pos++];
+    }
+    else
+    {
+        return buf[pos++];
+    }
 }
 
-//Reads some number of bytes from the input stream and stores them into the buffer array b.
+// Reads some number of bytes from the input stream and stores them into the buffer array b.
-//The number of bytes actually read is returned as an integer. This method blocks until input data is available,
+// The number of bytes actually read is returned as an integer. This method blocks until input data is available,
-//end of file is detected, or an exception is thrown.
+// end of file is detected, or an exception is thrown.
-//If the length of b is zero, then no bytes are read and 0 is returned; otherwise, there is an attempt to read at least one byte.
+// If the length of b is zero, then no bytes are read and 0 is returned; otherwise, there is an attempt to read at least one byte.
-//If no byte is available because the stream is at the end of the file, the value -1 is returned; otherwise,
+// If no byte is available because the stream is at the end of the file, the value -1 is returned; otherwise,
-//at least one byte is read and stored into b.
+// at least one byte is read and stored into b.
 //
-//The first byte read is stored into element b[0], the next one into b[1], and so on. The number of bytes read is,
+// The first byte read is stored into element b[0], the next one into b[1], and so on. The number of bytes read is,
-//at most, equal to the length of b. Let k be the number of bytes actually read; these bytes will be stored in elements b[0] through b[k-1],
+// at most, equal to the length of b. Let k be the number of bytes actually read; these bytes will be stored in elements b[0] through b[k-1],
-//leaving elements b[k] through b[b.length-1] unaffected.
+// leaving elements b[k] through b[b.length-1] unaffected.
 //
-//The read(b) method for class InputStream has the same effect as:
+// The read(b) method for class InputStream has the same effect as:
 //
-// read(b, 0, b.length)
+//  read(b, 0, b.length)
-//Parameters:
+// Parameters:
-//b - the buffer into which the data is read.
+// b - the buffer into which the data is read.
-//Returns:
+// Returns:
-//the total number of bytes read into the buffer, or -1 is there is no more data because the end of the stream has been reached.
+// the total number of bytes read into the buffer, or -1 is there is no more data because the end of the stream has been reached.
 int ByteArrayInputStream::read(byteArray b)
 {
-	return read( b, 0, b.length );
+    return read(b, 0, b.length);
 }
 
-//Reads up to len bytes of data into an array of bytes from this input stream. If pos equals count,
+// Reads up to len bytes of data into an array of bytes from this input stream. If pos equals count,
-//then -1 is returned to indicate end of file. Otherwise, the number k of bytes read is equal to the smaller of len and count-pos.
+// then -1 is returned to indicate end of file. Otherwise, the number k of bytes read is equal to the smaller of len and count-pos.
-//If k is positive, then bytes buf[pos] through buf[pos+k-1] are copied into b[off] through b[off+k-1] in the manner
+// If k is positive, then bytes buf[pos] through buf[pos+k-1] are copied into b[off] through b[off+k-1] in the manner
-//performed by System.arraycopy. The value k is added into pos and k is returned.
+// performed by System.arraycopy. The value k is added into pos and k is returned.
-//This read method cannot block.
+// This read method cannot block.
-//Parameters:
+// Parameters:
-//b - the buffer into which the data is read.
+// b - the buffer into which the data is read.
-//off - the start offset in the destination array b
+// off - the start offset in the destination array b
-//len - the maximum number of bytes read.
+// len - the maximum number of bytes read.
-//Returns:
+// Returns:
-//the total number of bytes read into the buffer, or -1 if there is no more data because the end of the stream has been reached.
+// the total number of bytes read into the buffer, or -1 if there is no more data because the end of the stream has been reached.
 int ByteArrayInputStream::read(byteArray b, unsigned int offset, unsigned int length)
 {
-	if( pos == count )
+    if (pos == count)
-		return -1;
+    {
+        return -1;
+    }
 
-	int k = min( length, count-pos );
+    int k = min(length, count - pos);
-	XMemCpy( &b[offset], &buf[pos], k );
+    XMemCpy(&b[offset], &buf[pos], k);
-	//std::copy( buf->data+pos, buf->data+pos+k, b->data + offset ); // Or this instead?
+    // std::copy( buf->data+pos, buf->data+pos+k, b->data + offset ); // Or this instead?
 
-	pos += k;
+    pos += k;
 
-	return k;
+    return k;
 }
 
-//Closing a ByteArrayInputStream has no effect.
+// Closing a ByteArrayInputStream has no effect.
-//The methods in this class can be called after the stream has been closed without generating an IOException.
+// The methods in this class can be called after the stream has been closed without generating an IOException.
 void ByteArrayInputStream::close()
 {
-	return;
+    return;
 }
 
-//Skips n bytes of input from this input stream. Fewer bytes might be skipped if the end of the input stream is reached. The actual number k of bytes to be skipped is equal to the smaller of n and count-pos. The value k is added into pos and k is returned.
+// Skips n bytes of input from this input stream. Fewer bytes might be skipped if the end of the input stream is reached. The actual number k of bytes to be skipped is equal to the smaller of n and count-pos. The value k is added into pos and k is returned.
-//Overrides:
+// Overrides:
-//skip in class InputStream
+// skip in class InputStream
-//Parameters:
+// Parameters:
-//n - the number of bytes to be skipped.
+// n - the number of bytes to be skipped.
-//Returns:
+// Returns:
-//the actual number of bytes skipped.
+// the actual number of bytes skipped.
-int64_t ByteArrayInputStream::skip(int64_t n)
+__int64 ByteArrayInputStream::skip(__int64 n)
 {
-	int newPos = pos + n;
+    int newPos = pos + n;
 
-	if(newPos > count) newPos = count;
+    if (newPos > count)
+    {
+        newPos = count;
+    }
 
-	int k = newPos - pos;
+    int k = newPos - pos;
-	pos = newPos;
+    pos = newPos;
 
-	return k;
+    return k;
 }
 
 ByteArrayInputStream::~ByteArrayInputStream()
 {
-	if(buf.data != nullptr) delete [] buf.data;
+    if (buf.data != NULL)
-}
+    {
+        delete[] buf.data;
+    }
+}

Minecraft.World/ByteArrayOutputStream.cpp

@@ -5,76 +5,95 @@
 // Creates a new byte array output stream. The buffer capacity is initially 32 bytes, though its size increases if necessary.
 ByteArrayOutputStream::ByteArrayOutputStream()
 {
-	count = 0;
+    count = 0;
-	buf = byteArray( 32 );
+    buf = byteArray(32);
 }
 
-//Creates a new byte array output stream, with a buffer capacity of the specified size, in bytes.
+// Creates a new byte array output stream, with a buffer capacity of the specified size, in bytes.
-//Parameters:
+// Parameters:
-//size - the initial size.
+// size - the initial size.
 ByteArrayOutputStream::ByteArrayOutputStream(unsigned int size)
 {
-	count = 0;
+    count = 0;
-	buf = byteArray( size );
+    buf = byteArray(size);
 }
 
 ByteArrayOutputStream::~ByteArrayOutputStream()
 {
-	if (buf.data != nullptr)
+    if (buf.data != NULL)
-		delete[] buf.data;
+    {
+        delete[] buf.data;
+    }
 }
 
-//Writes the specified byte to this byte array output stream.
+// Writes the specified byte to this byte array output stream.
-//Parameters:
+// Parameters:
-//b - the byte to be written.
+// b - the byte to be written.
 void ByteArrayOutputStream::write(unsigned int b)
 {
-	// If we will fill the buffer we need to make it bigger
+    // If we will fill the buffer we need to make it bigger
-	if( count + 1 >=  buf.length )
+    if (count + 1 >= buf.length)
-		buf.resize( buf.length * 2 );
+    {
+        buf.resize(buf.length * 2);
+    }
 
-	buf[count] = static_cast<byte>(b);
+    buf[count] = (byte)b;
-	count++;
+    count++;
 }
 
 // Writes b.length bytes from the specified byte array to this output stream.
-//The general contract for write(b) is that it should have exactly the same effect as the call write(b, 0, b.length).
+// The general contract for write(b) is that it should have exactly the same effect as the call write(b, 0, b.length).
 void ByteArrayOutputStream::write(byteArray b)
 {
-	write(b, 0, b.length);
+    write(b, 0, b.length);
 }
 
-//Writes len bytes from the specified byte array starting at offset off to this byte array output stream.
+// Writes len bytes from the specified byte array starting at offset off to this byte array output stream.
-//Parameters:
+// Parameters:
-//b - the data.
+// b - the data.
-//off - the start offset in the data.
+// off - the start offset in the data.
-//len - the number of bytes to write.
+// len - the number of bytes to write.
 void ByteArrayOutputStream::write(byteArray b, unsigned int offset, unsigned int length)
 {
-	assert( b.length >= offset + length );
 
-	// If we will fill the buffer we need to make it bigger
+    if (offset > b.length || length > b.length - offset)
-	if( count + length >=  buf.length )
+    {
-		buf.resize( max( count + length + 1, buf.length * 2 ) );
+        return;
+    }
 
-	XMemCpy( &buf[count], &b[offset], length );
+    if (length > 0xFFFFFFFF - count)
-	//std::copy( b->data+offset, b->data+offset+length, buf->data + count ); // Or this instead?
+    {
+        return;
+    }
 
-	count += length;
+    // If we will fill the buffer we need to make it bigger
+    if (count + length >= buf.length)
+    {
+        unsigned int newSize = (std::max)(count + length + 1, buf.length * 2);
+        if (newSize <= buf.length)
+        {
+            return;
+        }
+        buf.resize(newSize);
+    }
+
+    XMemCpy(&buf[count], &b[offset], length);
+
+    count += length;
 }
 
-//Closing a ByteArrayOutputStream has no effect.
+// Closing a ByteArrayOutputStream has no effect.
-//The methods in this class can be called after the stream has been closed without generating an IOException.
+// The methods in this class can be called after the stream has been closed without generating an IOException.
 void ByteArrayOutputStream::close()
 {
 }
 
-//Creates a newly allocated byte array. Its size is the current size of this output stream and the valid contents of the buffer have been copied into it.
+// Creates a newly allocated byte array. Its size is the current size of this output stream and the valid contents of the buffer have been copied into it.
-//Returns:
+// Returns:
-//the current contents of this output stream, as a byte array.
+// the current contents of this output stream, as a byte array.
 byteArray ByteArrayOutputStream::toByteArray()
 {
-	byteArray out(count);
+    byteArray out(count);
-	memcpy(out.data,buf.data,count);
+    memcpy(out.data, buf.data, count);
-	return out;
+    return out;
-}
+}

Minecraft.World/ByteArrayTag.h

@@ -21,6 +21,7 @@ public:
 	void load(DataInput *dis, int tagDepth)
 	{
 		int length = dis->readInt();
+        if (length < 0 || length > 2 * 1024 * 1024) length = 0;
 		
 		if ( data.data ) delete[] data.data;
 		data  = byteArray(length);

Minecraft.World/ComplexItemDataPacket.cpp

@@ -32,7 +32,12 @@ void ComplexItemDataPacket::read(DataInputStream *dis) //throws IOException
 	itemType = dis->readShort();
 	itemId = dis->readShort();
 
-	data = charArray(dis->readUnsignedShort() & 0xffff);
+	int dataLength = dis->readShort() & 0xffff;
+    if (dataLength > 32767)
+    {
+        dataLength = 0;
+    }
+    data = charArray(dataLength);
 	dis->readFully(data);
 }
 

Minecraft.World/CompoundTag.h

@@ -42,10 +42,16 @@ public:
 		}
 		tags.clear();
 		Tag *tag;
-		while ((tag = Tag::readNamedTag(dis))->getId() != Tag::TAG_End)
+        int tagCount = 0;
-		{
+        const int MAX_COMPOUND_TAGS = 10000;
-			tags[tag->getName()] = tag;
+        while ((tag = Tag::readNamedTag(dis))->getId() != Tag::TAG_End)
-		}
+        {
+            tags[tag->getName()] = tag;
+            if (++tagCount >= MAX_COMPOUND_TAGS)
+            {
+                break;
+            }
+        }
 		delete tag;
 	}
 

Minecraft.World/Connection.cpp

@@ -108,8 +108,8 @@ Connection::Connection(Socket *socket, const wstring& id, PacketListener *packet
 	const char *szId = wstringtofilename(id);
 	char readThreadName[256];
 	char writeThreadName[256];
-	sprintf(readThreadName,"%s read\n",szId);
+    sprintf_s(readThreadName, sizeof(readThreadName), "%.240s read\n", szId);
-	sprintf(writeThreadName,"%s write\n",szId);
+    sprintf_s(writeThreadName, sizeof(writeThreadName), "%.240s write\n", szId);
 
 	readThread = new C4JThread(runRead, static_cast<void *>(this), readThreadName, READ_STACK_SIZE);
 	writeThread = new C4JThread(runWrite, this, writeThreadName, WRITE_STACK_SIZE);

Minecraft.World/ContainerSetContentPacket.cpp

@@ -32,6 +32,9 @@ void ContainerSetContentPacket::read(DataInputStream *dis) //throws IOException
 {
 	containerId = dis->readByte();
 	int count = dis->readShort();
+
+	if (count < 0 || count > 256) count = 0;
+
 	items = ItemInstanceArray(count);
 	for (int i = 0; i < count; i++) 
 	{

Minecraft.World/ContainerSetSlotPacket.cpp

@@ -35,7 +35,7 @@ void ContainerSetSlotPacket::read(DataInputStream *dis) //throws IOException
 	// 4J Stu - TU-1 hotfix
 	// Fix for #13142 - Holding down the A button on the furnace ingredient slot causes the UI to display incorrect item counts
 	BYTE byteId = dis->readByte();
-	containerId = *(char *)&byteId;
+    containerId = (char)(signed char)byteId;
 	slot = dis->readShort();
 	item = readItem(dis);
 }

Minecraft.World/CustomPayloadPacket.cpp

@@ -43,7 +43,7 @@ void CustomPayloadPacket::read(DataInputStream *dis)
 	identifier = readUtf(dis, 20);
 	length = dis->readShort();
 
-	if (length > 0 && length < Short::MAX_VALUE)
+	if (length > 0 && length <= Short::MAX_VALUE)
 	{
 		if(data.data != nullptr)
 		{

Minecraft.World/DataInputStream.cpp

@@ -303,6 +303,10 @@ wstring DataInputStream::readUTF()
 	int b = stream->read();
 	unsigned short UTFLength = static_cast<unsigned short>(((a & 0xff) << 8) | (b & 0xff));
 
+	const unsigned short MAX_UTF_LENGTH = 32767;
+    if (UTFLength > MAX_UTF_LENGTH)
+        return outputString;
+
 	//// 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
 

Minecraft.World/ExplodePacket.cpp

@@ -52,6 +52,8 @@ void ExplodePacket::read(DataInputStream *dis) //throws IOException
 		r = dis->readFloat();
 		int count = dis->readInt();
 
+		if (count < 0 || count > 32768) count = 0;
+
 		int xp = static_cast<int>(x);
 		int yp = static_cast<int>(y);
 		int zp = static_cast<int>(z);

Minecraft.World/GameCommandPacket.cpp

@@ -40,7 +40,7 @@ void GameCommandPacket::read(DataInputStream *dis)
 	command = static_cast<EGameCommand>(dis->readInt());
 	length = dis->readShort();
 
-	if (length > 0 && length < Short::MAX_VALUE)
+	if (length > 0 && length <= Short::MAX_VALUE)
 	{
 		if(data.data != nullptr)
 		{

Minecraft.World/IntArrayTag.h

@@ -35,6 +35,8 @@ public:
 	void load(DataInput *dis, int tagDepth)
 	{
 		int length = dis->readInt();
+        if (length < 0 || length > 65536)
+			length = 0;
 
 		if ( data.data ) delete[] data.data;
 		data = intArray(length);

Minecraft.World/ListTag.h

@@ -26,21 +26,16 @@ public:
 
 	void load(DataInput *dis, int tagDepth)
 	{
-		if (tagDepth > MAX_DEPTH)
-		{
-#ifndef _CONTENT_PACKAGE
-			printf("Tried to read NBT tag with too high complexity, depth > %d", MAX_DEPTH);
-			__debugbreak();
-#endif
-			return;
-		}
 		type = dis->readByte();
 		int size = dis->readInt();
+        if (size < 0 || size > MAX_DEPTH)
+			size = 0;
 
 		list.clear();
 		for (int i = 0; i < size; i++)
 		{
 			Tag *tag = Tag::newTag(type, L"");
+            if (tag == nullptr) break;
 			tag->load(dis, tagDepth);
 			list.push_back(tag);
 		}

Minecraft.World/Packet.cpp

@@ -267,8 +267,12 @@ void Packet::updatePacketStatsPIX()
 
 shared_ptr<Packet> Packet::getPacket(int id)
 {
-	// 4J: Removed try/catch
+    auto it = idToCreateMap.find(id);
-	return idToCreateMap[id]();
+    if (it == idToCreateMap.end())
+    {
+        return nullptr;
+    }
+    return it->second();
 }
 
 void Packet::writeBytes(DataOutputStream *dataoutputstream, byteArray bytes)
@@ -334,30 +338,11 @@ shared_ptr<Packet> 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 (0x%02X) isServer=%d totalPacketsRead=%d\n", id, id, isServer ? 1 : 0, s_packetCount);
+        return nullptr;
-		app.DebugPrintf("*** Last %d good packet IDs (oldest first): ", 8);
-		for (int dbg = 0; dbg < 8; dbg++)
-		{
-			int idx = (s_lastIdPos + dbg) % 8;
-			app.DebugPrintf("%d ", s_lastIds[idx]);
-		}
-		app.DebugPrintf("\n");
-		// Dump the next 32 bytes from the stream to see what follows
-		app.DebugPrintf("*** Next bytes in stream: ");
-		for (int dbg = 0; dbg < 32; dbg++)
-		{
-			int b = dis->read();
-			if (b == -1) { app.DebugPrintf("[EOS] "); break; }
-			app.DebugPrintf("%02X ", b);
-		}
-		app.DebugPrintf("\n");
-		__debugbreak();
-		assert(false);
-		//            throw new IOException(wstring(L"Bad packet id ") + std::to_wstring(id));
 	}
 
 	packet = getPacket(id);
-	if (packet == nullptr) assert(false);//throw new IOException(wstring(L"Bad packet id ") + std::to_wstring(id));
+	if (packet == nullptr) return nullptr;//throw new IOException(wstring(L"Bad packet id ") + std::to_wstring(id));
 
 	s_lastIds[s_lastIdPos] = id;
 	s_lastIdPos = (s_lastIdPos + 1) % 8;
@@ -418,11 +403,9 @@ wstring Packet::readUtf(DataInputStream *dis, int maxLength) // throws IOExcepti
 {
 
 	short stringLength = dis->readShort();
-	if (stringLength > maxLength)
+	if (stringLength > maxLength || stringLength <= 0)
 	{
-		wstringstream stream;
+        return L"";
-		stream << L"Received string length longer than maximum allowed (" << stringLength << " > " << maxLength << ")";
-		assert(false);
 		//        throw new IOException( stream.str() );
 	}
 	if (stringLength < 0)
@@ -531,7 +514,7 @@ shared_ptr<ItemInstance> Packet::readItem(DataInputStream *dis)
 {
 	shared_ptr<ItemInstance> item = nullptr;
 	int id = dis->readShort();
-	if (id >= 0)
+    if (id >= 0 && id < 32000) // todo: should turn Item::ITEM_NUM_COUNT into a global define 
 	{
 		int count = dis->readByte();
 		int damage = dis->readShort();
@@ -569,9 +552,16 @@ void Packet::writeItem(shared_ptr<ItemInstance> item, DataOutputStream *dos)
 CompoundTag *Packet::readNbt(DataInputStream *dis)
 {
 	int size = dis->readShort();
-	if (size < 0) return nullptr;
+	if (size <= 0) return nullptr;
+
+	const int MAX_NBT_SIZE = 32767;
+	if (size > MAX_NBT_SIZE) return nullptr;
 	byteArray buff(size);
-	dis->readFully(buff);
+	if (!dis->readFully(buff))
+	{
+		delete [] buff.data;
+        return nullptr;
+	}
 	CompoundTag *result = (CompoundTag *) NbtIo::decompress(buff);
 	delete [] buff.data;
 	return result;

Minecraft.World/PreLoginPacket.cpp

@@ -62,6 +62,7 @@ void PreLoginPacket::read(DataInputStream *dis) //throws IOException
 	m_friendsOnlyBits = dis->readByte();
 	m_ugcPlayersVersion = dis->readInt();
 	m_dwPlayerCount = dis->readByte();
+	if( m_dwPlayerCount > MINECRAFT_NET_MAX_PLAYERS ) m_dwPlayerCount = MINECRAFT_NET_MAX_PLAYERS;
 	if( m_dwPlayerCount > 0 )
 	{
 		m_playerXuids = new PlayerUID[m_dwPlayerCount];
@@ -74,6 +75,7 @@ void PreLoginPacket::read(DataInputStream *dis) //throws IOException
 	{
 		m_szUniqueSaveName[i]=dis->readByte();
 	}
+	// m_szUniqueSaveName[m_iSaveNameLen - 1] = 0; // LCEMP does this but I have no idea why, TODO: why?
 	m_serverSettings = dis->readInt();
 	m_hostIndex = dis->readByte();
 	

Minecraft.World/RemoveEntitiesPacket.cpp

@@ -21,7 +21,9 @@ RemoveEntitiesPacket::~RemoveEntitiesPacket()
 
 void RemoveEntitiesPacket::read(DataInputStream *dis) //throws IOException 
 {
-	ids = intArray(dis->readByte());
+	int count = dis->readByte();
+	if(count < 0) count = 0;
+	ids = intArray(count);
 	for(unsigned int i = 0; i < ids.length; ++i)
 	{
 		ids[i] = dis->readInt();

Minecraft.World/Socket.cpp

@@ -141,6 +141,11 @@ void Socket::pushDataToQueue(const BYTE * pbData, DWORD dwDataSize, bool fromHos
 	//	dwDataSize, queueIdx, dwDataSize > 0 ? pbData[0] : 0, networkPlayerSmallId);
 
 	EnterCriticalSection(&m_queueLockNetwork[queueIdx]);
+    if (m_queueNetwork[queueIdx].size() + dwDataSize > 2 * 1024 * 1024)
+    {
+        LeaveCriticalSection(&m_queueLockNetwork[queueIdx]);
+        return;
+    }
 	for( unsigned int i = 0; i < dwDataSize; i++ )
 	{
 		m_queueNetwork[queueIdx].push(*pbData++);

Minecraft.World/SynchedEntityData.cpp

@@ -342,7 +342,10 @@ vector<shared_ptr<SynchedEntityData::DataItem> > *SynchedEntityData::unpack(Data
 
 	int currentHeader = input->readByte();
 
-	while (currentHeader != EOF_MARKER)
+	int itemCount = 0;
+    const int MAX_ENTITY_DATA_ITEMS = 256;
+
+    while (currentHeader != EOF_MARKER && itemCount < MAX_ENTITY_DATA_ITEMS)
 	{
 
 		if (result == nullptr)
@@ -397,6 +400,7 @@ vector<shared_ptr<SynchedEntityData::DataItem> > *SynchedEntityData::unpack(Data
 			break;
 		}
 		result->push_back(item);
+        itemCount++;
 
 		currentHeader = input->readByte();
 	}

Minecraft.World/Tag.cpp

@@ -84,27 +84,56 @@ Tag *Tag::readNamedTag(DataInput *dis)
 
 Tag *Tag::readNamedTag(DataInput *dis, int tagDepth)
 {
+    static __declspec(thread) int depth = 0;
+    static __declspec(thread) int totalTagCount = 0;
+
+    if (depth == 0)
+    {
+        totalTagCount = 0;
+    }
+
+    depth++;
+
+    if (depth > 256)
+    {
+        depth--;
+        return new EndTag();
+    }
+
+    totalTagCount++;
+    const int MAX_TOTAL_TAGS = 32768;
+    if (totalTagCount > MAX_TOTAL_TAGS)
+    {
+        depth--;
+        return new EndTag();
+    }
+
 	byte type = dis->readByte();
-	if (type == 0) return new EndTag();
+	if (type == 0) { 
+		depth--; 
+		return new EndTag(); 
+	}
 
 	// 4J Stu - readByte can return -1, so if it's that then also mark as the end tag
 	if(type == 255)
 	{
-		app.DebugPrintf("readNamedTag read a type of 255\n");
+        depth--;
-#ifndef _CONTENT_PACKAGE
-		__debugbreak();
-#endif
 		return new EndTag();
 	}
 
 	wstring name = dis->readUTF();//new String(bytes, "UTF-8");
 
 	Tag *tag = newTag(type, name);
+	if (tag == nullptr) { 
+		depth--; 
+		return new EndTag();
+	}
 	//        short length = dis.readShort();
 	//        byte[] bytes = new byte[length];
 	//        dis.readFully(bytes);
 
 	tag->load(dis, tagDepth);
+    depth--;
 	return tag;
 }
 

Minecraft.World/TextureAndGeometryPacket.cpp

@@ -121,7 +121,20 @@ void TextureAndGeometryPacket::read(DataInputStream *dis) //throws IOException
 {
 	textureName = dis->readUTF();
 	dwSkinID = static_cast<DWORD>(dis->readInt());
-	dwTextureBytes = static_cast<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 +147,19 @@ void TextureAndGeometryPacket::read(DataInputStream *dis) //throws IOException
 	}
 	uiAnimOverrideBitmask = dis->readInt();
 
-	dwBoxC = static_cast<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)
 	{

Minecraft.World/TexturePacket.cpp

@@ -37,17 +37,26 @@ void TexturePacket::handle(PacketListener *listener)
 void TexturePacket::read(DataInputStream *dis) //throws IOException
 {
 	textureName = dis->readUTF();
-	dwBytes = static_cast<DWORD>(dis->readShort());
+    short rawBytes = dis->readShort();
+    if (rawBytes <= 0)
+    {
+        dwBytes = 0;
+        return;
+    }
+    dwBytes = (DWORD)(unsigned short)rawBytes;
+    if (dwBytes > 65536)
+    {
+        dwBytes = 0;
+        return;
+    }
+	
+	this->pbData= new BYTE [dwBytes];
 
-	if(dwBytes>0)
+	for(DWORD i=0;i<dwBytes;i++)
 	{
-		this->pbData= new BYTE [dwBytes];
+		this->pbData[i] = dis->readByte();
-
-		for(DWORD i=0;i<dwBytes;i++)
-		{
-			this->pbData[i] = dis->readByte();
-		}
 	}
+	
 }
 
 void TexturePacket::write(DataOutputStream *dos) //throws IOException 

Minecraft.World/ThreadName.cpp

@@ -22,9 +22,9 @@ void SetThreadName( DWORD dwThreadID, LPCSTR szThreadName )
 #if ( defined _WINDOWS64 | defined _DURANGO )
 	__try
 	{
-		RaiseException( 0x406D1388, 0, sizeof(info)/sizeof(DWORD), (ULONG_PTR *)&info );
+        RaiseException(0x406D1388, 0, sizeof(info)/sizeof(ULONG_PTR), (ULONG_PTR *)&info);
-	}
+    }
-	__except( GetExceptionCode()==0x406D1388 ? EXCEPTION_CONTINUE_EXECUTION : EXCEPTION_EXECUTE_HANDLER )
+    __except (EXCEPTION_EXECUTE_HANDLER)
 	{
 	}
 #endif

Minecraft.World/UpdateGameRuleProgressPacket.cpp

@@ -18,7 +18,7 @@ UpdateGameRuleProgressPacket::UpdateGameRuleProgressPacket(ConsoleGameRules::EGa
 	m_auxValue = auxValue;
 	m_dataTag = dataTag;
 
-	if(dataLength > 0)
+	if (dataLength > 0 && dataLength <= 65536)
 	{
 		m_data = byteArray(dataLength);
 		memcpy(m_data.data,data,dataLength);

Minecraft.World/compression.cpp

@@ -237,9 +237,19 @@ HRESULT Compression::DecompressLZXRLE(void *pDestination, unsigned int *pDestSiz
 	unsigned char *dynamicRleBuf = nullptr;
 	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)
+    {
+        LeaveCriticalSection(&rleDecompressLock);
+        *pDestSize = 0;
+        return E_FAIL;
+    }
+
+    if (safeRleSize > staticRleSize)
 	{
-		rleSize = *pDestSize;
+        rleSize = safeRleSize;
 		dynamicRleBuf = new unsigned char[rleSize];
 		decompressResult = Decompress(dynamicRleBuf, &rleSize, pSource, SrcSize);
 		pucIn = (unsigned char *)dynamicRleBuf;
@@ -263,7 +273,7 @@ HRESULT Compression::DecompressLZXRLE(void *pDestination, unsigned int *pDestSiz
 	//unsigned char *pucIn = (unsigned char *)rleDecompressBuf;
 	const unsigned char *pucEnd = pucIn + rleSize;
 	unsigned char *pucOut = static_cast<unsigned char*>(pDestination);
-	const unsigned char *pucOutEnd = pucOut + *pDestSize;
+	unsigned char *pucOutEnd = pucOut + *pDestSize;
 
 	while( pucIn != pucEnd )
 	{
@@ -275,7 +285,11 @@ HRESULT Compression::DecompressLZXRLE(void *pDestination, unsigned int *pDestSiz
 			if( count < 3 )
 			{
 				count++;
-				if( pucOut + count > pucOutEnd ) break;
+                if (pucOut + count > pucOutEnd)
+                {
+                    pucOut = pucOutEnd;
+                    break;
+                }
 				for( unsigned int i = 0; i < count; i++ )
 				{
 					*pucOut++ = 255;
@@ -286,7 +300,11 @@ HRESULT Compression::DecompressLZXRLE(void *pDestination, unsigned int *pDestSiz
 				count++;
 				if( pucIn >= pucEnd ) break;
 				const unsigned char data = *pucIn++;
-				if( pucOut + count > pucOutEnd ) break;
+                if (pucOut + count > pucOutEnd)
+                {
+                    pucOut = pucOutEnd;
+                    break;
+                }
 				for( unsigned int i = 0; i < count; i++ )
 				{
 					*pucOut++ = data;
@@ -317,7 +335,7 @@ HRESULT Compression::DecompressRLE(void *pDestination, unsigned int *pDestSize,
 	unsigned char *pucIn  = static_cast<unsigned char *>(pSource);
 	const unsigned char *pucEnd = pucIn + SrcSize;
 	unsigned char *pucOut = static_cast<unsigned char*>(pDestination);
-	const unsigned char *pucOutEnd = pucOut + *pDestSize;
+	unsigned char *pucOutEnd = pucOut + *pDestSize;
 
 	while( pucIn != pucEnd )
 	{
@@ -329,7 +347,11 @@ HRESULT Compression::DecompressRLE(void *pDestination, unsigned int *pDestSize,
 			if( count < 3 )
 			{
 				count++;
-				if( pucOut + count > pucOutEnd ) break;
+                if (pucOut + count > pucOutEnd)
+                {
+                    pucOut = pucOutEnd;
+                    break;
+                }
 				for( unsigned int i = 0; i < count; i++ )
 				{
 					*pucOut++ = 255;
@@ -340,7 +362,11 @@ HRESULT Compression::DecompressRLE(void *pDestination, unsigned int *pDestSize,
 				count++;
 				if( pucIn >= pucEnd ) break;
 				const unsigned char data = *pucIn++;
-				if( pucOut + count > pucOutEnd ) break;
+                if (pucOut + count > pucOutEnd)
+                {
+                    pucOut = pucOutEnd;
+                    break;
+                }
 				for( unsigned int i = 0; i < count; i++ )
 				{
 					*pucOut++ = data;