Sanitise texture item IDs for facades #7905
Conversation
| ResourceLocation itemId; | ||
|
|
||
| try { | ||
| itemId = new ResourceLocation(nbt.getString(NBT_ITEM_ID)); | ||
| } catch (ResourceLocationException e) { | ||
| return ItemStack.EMPTY; | ||
| } |
There was a problem hiding this comment.
@62832 Do you know what the resource-id is that they get crashes for?
I have a hunch it's actually just an empty string since we don't guard against a tag being present, but it not having our proprty... (giving us an empty string)
There was a problem hiding this comment.
Anything malformed with a non A-Z such as minecraft::dirt will cause a crash otherwise.
There was a problem hiding this comment.
Yes but that is arbitrarily edited NBT which I don't really care about :D
There was a problem hiding this comment.
You can crash half of AE2 if you just corrupt the NBT entirely
There was a problem hiding this comment.
The entire reason this PR was made is because a large server already reported someone using an exploit to obtain arbitrarily-modified items and cause client crashes for other players, leading to the modpack for this server disabling facades entirely.
There was a problem hiding this comment.
And you are certain this was not due the item having any NBT?
Because our code actually also crashes if the item has a tag, but the tag is missing or property...
There was a problem hiding this comment.
And this is 100% not the only place we do this:
There was a problem hiding this comment.
I didn't say the item had no NBT at all. This was because the item NBT was present on the facade but the value it actually held was malformed.
Forge counterpart to #7905. --------- Co-authored-by: Sebastian Hartte <shartte@users.noreply.github.com>
Fixes an instant crash bug reported by the Prominence 2 modpack devs if a facade has somehow been obtained with a malformed texture item ID.