Tiviacz1337 / PizzaCraft

PizzaCraft
8 stars 16 forks source link

Pizza recursion overloads NBT #103

Closed fengshuo2004 closed 4 months ago

fengshuo2004 commented 11 months ago

Description

It turns out that this mod and Some Assembly Required both saves the full NBT data of ingredients inside the pizza/sandwich items. This means by putting a pizza inside a sandwich inside a pizza inside a sandwich inside a pizza... you obtain an item with gigantic amount of NBT data that would crash the client as soon as it's placed into a tile entity.

image

A friend on my server found this out the hard way. At this point, the pizza is inside a pizza station. Anyone close enough to load that chunk is kicked instantly and couldn't rejoin. This is effectively a chunk ban. We had to reset the entity data of the pizza station using a NBT editor.

PizzaBomb

Here's a picture of the sandwich one step before wreaking havoc:

2023-09-24_16 26 16

To reproduce

  1. Create sandwich using the sandwich station (from mod Some Assembly Required)
  2. Put sandwiches into pizza station's ingredient slots
  3. Create and cook the pizza
  4. Slice the pizza and put the slices inside another sandwich
  5. Repeat 2-4 until 9 sandwiches
  6. Place back into the pizza station, you are instantly kicked along other players in chunk loading range

Potential Fix

IMO the best fix would be to store the ID of ingredients inside the pizza without copying all their NBT data.

I've had a look at your code, in this function:

https://github.com/Tiviacz1337/PizzaCraft/blob/6d5bda0bbe957dffb1756bc1753107b36bef3aa6/src/main/java/com/tiviacz/pizzacraft/common/PizzaCalculator.java#L38

It calls NBTUtils.saveInventoryToStack() which seems to do a deep copy of the ingredients' data. You might want to change this to only copy the item ID instead.

Thanks.

Tiviacz1337 commented 5 months ago

blacklisted sandwitch from some assembly required in pizza

Tiviacz1337 commented 4 months ago

https://www.curseforge.com/minecraft/mc-mods/pizzacraft/files/all?page=1&pageSize=20

fengshuo2004 commented 1 month ago

@Tiviacz1337 Hi. Please re-open this issue. It turns out Some Assembly Required is not the cause here. You can still crash client side by putting pizza inside a pizza inside a pizza inside a pizza... The NBT data of pizza ingredients MUST be sanitized to properly fix this.