From 4939f8711884901ddf1c56337f606de71cdae78d Mon Sep 17 00:00:00 2001 From: Nassim Jahnke Date: Sun, 18 Feb 2024 12:53:27 +0100 Subject: [PATCH] Add more ResourceLocation checks, some cleanup --- ...able-API-and-replenishable-lootables.patch | 4 +- ...CanPlaceOn-and-CanDestroy-NBT-values.patch | 33 ++++----- ...low-bees-to-load-chunks-for-beehives.patch | 16 +++++ ...date-ResourceLocation-in-NBT-reading.patch | 33 ++++++++- .../1039-Improve-tag-parser-handling.patch | 68 +++++++++++++------ 5 files changed, 111 insertions(+), 43 deletions(-) diff --git a/patches/server/0092-LootTable-API-and-replenishable-lootables.patch b/patches/server/0092-LootTable-API-and-replenishable-lootables.patch index 2f441da2cc..2c728e93f8 100644 --- a/patches/server/0092-LootTable-API-and-replenishable-lootables.patch +++ b/patches/server/0092-LootTable-API-and-replenishable-lootables.patch @@ -654,7 +654,7 @@ index e0fbacd574e0c83c2e1d164ded8e9ccf4af30480..7529751afa2932fd16bc4591189b0358 + // Paper end } diff --git a/src/main/java/net/minecraft/world/level/block/entity/RandomizableContainerBlockEntity.java b/src/main/java/net/minecraft/world/level/block/entity/RandomizableContainerBlockEntity.java -index aa4181e59f88be04a3605352fa5ceb3e04149dd3..7cbd403f9e96e7ce35475c8102cd9f9c04819c27 100644 +index aa4181e59f88be04a3605352fa5ceb3e04149dd3..e4e827a57c2913c719282cc0d5da33586607677b 100644 --- a/src/main/java/net/minecraft/world/level/block/entity/RandomizableContainerBlockEntity.java +++ b/src/main/java/net/minecraft/world/level/block/entity/RandomizableContainerBlockEntity.java @@ -17,6 +17,7 @@ public abstract class RandomizableContainerBlockEntity extends BaseContainerBloc @@ -675,7 +675,7 @@ index aa4181e59f88be04a3605352fa5ceb3e04149dd3..7cbd403f9e96e7ce35475c8102cd9f9c + // Copied from super with changes, always check the original method + this.lootableData.loadNbt(nbt); // Paper + if (nbt.contains("LootTable", 8)) { -+ this.setLootTable(new ResourceLocation(nbt.getString("LootTable"))); ++ this.setLootTable(ResourceLocation.tryParse(nbt.getString("LootTable"))); + try { org.bukkit.craftbukkit.util.CraftNamespacedKey.fromMinecraft(this.lootTable); } catch (IllegalArgumentException ex) { this.lootTable = null; } // Paper - validate + this.setLootTableSeed(nbt.getLong("LootTableSeed")); + return false; // Paper - always load the items, table may still remain diff --git a/patches/server/0248-Add-API-for-CanPlaceOn-and-CanDestroy-NBT-values.patch b/patches/server/0248-Add-API-for-CanPlaceOn-and-CanDestroy-NBT-values.patch index 61c1bd0e6e..f179b48bd7 100644 --- a/patches/server/0248-Add-API-for-CanPlaceOn-and-CanDestroy-NBT-values.patch +++ b/patches/server/0248-Add-API-for-CanPlaceOn-and-CanDestroy-NBT-values.patch @@ -5,7 +5,7 @@ Subject: [PATCH] Add API for CanPlaceOn and CanDestroy NBT values diff --git a/src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaItem.java b/src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaItem.java -index 1920cf7ad846f57cd278cb9a72dce03f3d014fbb..629fa76e6c7c2ede36ab855bb3a7a65dfd601449 100644 +index 1920cf7ad846f57cd278cb9a72dce03f3d014fbb..7cf1153ae532a9d53ee85b05f77ed74b94cf5fbc 100644 --- a/src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaItem.java +++ b/src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaItem.java @@ -85,6 +85,12 @@ import org.bukkit.persistence.PersistentDataContainer; @@ -67,7 +67,7 @@ index 1920cf7ad846f57cd278cb9a72dce03f3d014fbb..629fa76e6c7c2ede36ab855bb3a7a65d + if (tag.contains(CAN_DESTROY.NBT)) { + ListTag list = tag.getList(CAN_DESTROY.NBT, CraftMagicNumbers.NBT.TAG_STRING); + for (int i = 0; i < list.size(); i++) { -+ Namespaced namespaced = this.deserializeNamespaced(list.getString(i)); ++ Namespaced namespaced = this.blockKeyFromString(list.getString(i)); + if (namespaced == null) { + continue; + } @@ -79,7 +79,7 @@ index 1920cf7ad846f57cd278cb9a72dce03f3d014fbb..629fa76e6c7c2ede36ab855bb3a7a65d + if (tag.contains(CAN_PLACE_ON.NBT)) { + ListTag list = tag.getList(CAN_PLACE_ON.NBT, CraftMagicNumbers.NBT.TAG_STRING); + for (int i = 0; i < list.size(); i++) { -+ Namespaced namespaced = this.deserializeNamespaced(list.getString(i)); ++ Namespaced namespaced = this.blockKeyFromString(list.getString(i)); + if (namespaced == null) { + continue; + } @@ -100,7 +100,7 @@ index 1920cf7ad846f57cd278cb9a72dce03f3d014fbb..629fa76e6c7c2ede36ab855bb3a7a65d + if (canPlaceOnSerialized != null) { + for (Object canPlaceOnElement : canPlaceOnSerialized) { + String canPlaceOnRaw = (String) canPlaceOnElement; -+ Namespaced value = this.deserializeNamespaced(canPlaceOnRaw); ++ Namespaced value = this.blockKeyFromString(canPlaceOnRaw); + if (value == null) { + continue; + } @@ -113,7 +113,7 @@ index 1920cf7ad846f57cd278cb9a72dce03f3d014fbb..629fa76e6c7c2ede36ab855bb3a7a65d + if (canDestroySerialized != null) { + for (Object canDestroyElement : canDestroySerialized) { + String canDestroyRaw = (String) canDestroyElement; -+ Namespaced value = this.deserializeNamespaced(canDestroyRaw); ++ Namespaced value = this.blockKeyFromString(canDestroyRaw); + if (value == null) { + continue; + } @@ -253,7 +253,7 @@ index 1920cf7ad846f57cd278cb9a72dce03f3d014fbb..629fa76e6c7c2ede36ab855bb3a7a65d // Paper end CraftMetaCompass.LODESTONE_DIMENSION.NBT, CraftMetaCompass.LODESTONE_POS.NBT, -@@ -1545,4 +1688,146 @@ class CraftMetaItem implements ItemMeta, Damageable, Repairable, BlockDataMeta { +@@ -1545,4 +1688,141 @@ class CraftMetaItem implements ItemMeta, Damageable, Repairable, BlockDataMeta { } // Paper end @@ -347,8 +347,8 @@ index 1920cf7ad846f57cd278cb9a72dce03f3d014fbb..629fa76e6c7c2ede36ab855bb3a7a65d + return mats; + } + -+ private @Nullable Namespaced deserializeNamespaced(String raw) { -+ boolean isTag = raw.length() > 0 && raw.codePointAt(0) == '#'; ++ private @Nullable Namespaced blockKeyFromString(String raw) { ++ boolean isTag = !raw.isEmpty() && raw.codePointAt(0) == '#'; + com.mojang.datafixers.util.Either result; + try { + result = net.minecraft.commands.arguments.blocks.BlockStateParser.parseForTesting(net.minecraft.core.registries.BuiltInRegistries.BLOCK.asLookup(), raw, false); @@ -367,20 +367,15 @@ index 1920cf7ad846f57cd278cb9a72dce03f3d014fbb..629fa76e6c7c2ede36ab855bb3a7a65d + return null; + } + -+ // don't DC the player if something slips through somehow -+ Namespaced resource = null; + try { + if (isTag) { -+ resource = new NamespacedTag(key.getNamespace(), key.getPath()); -+ } else { -+ resource = CraftNamespacedKey.fromMinecraft(key); -+ } -+ } catch (IllegalArgumentException ex) { -+ org.bukkit.Bukkit.getLogger().warning("Namespaced resource does not validate: " + key.toString()); -+ ex.printStackTrace(); -+ } ++ return new NamespacedTag(key.getNamespace(), key.getPath()); + -+ return resource; ++ } ++ return CraftNamespacedKey.fromMinecraft(key); ++ } catch (IllegalArgumentException ignored) { ++ return null; ++ } + } + + private @Nonnull String serializeNamespaced(Namespaced resource) { diff --git a/patches/server/0342-Do-not-allow-bees-to-load-chunks-for-beehives.patch b/patches/server/0342-Do-not-allow-bees-to-load-chunks-for-beehives.patch index 9c629a712d..79bb156283 100644 --- a/patches/server/0342-Do-not-allow-bees-to-load-chunks-for-beehives.patch +++ b/patches/server/0342-Do-not-allow-bees-to-load-chunks-for-beehives.patch @@ -40,3 +40,19 @@ index 06e990b6957a4ef48c8778bbd175d3afddf52ca3..07ecc038a1000581335b8d18c094298f BlockEntity tileentity = Bee.this.level().getBlockEntity(Bee.this.hivePos); if (tileentity instanceof BeehiveBlockEntity) { +diff --git a/src/main/java/net/minecraft/world/entity/monster/Vex.java b/src/main/java/net/minecraft/world/entity/monster/Vex.java +index 30ea3f64234fd1fda8dada3c7fb12be0730322a8..f443006c1e32feee97b32312814e2447a50c45e2 100644 +--- a/src/main/java/net/minecraft/world/entity/monster/Vex.java ++++ b/src/main/java/net/minecraft/world/entity/monster/Vex.java +@@ -377,7 +377,10 @@ public class Vex extends Monster implements TraceableEntity { + for (int i = 0; i < 3; ++i) { + BlockPos blockposition1 = blockposition.offset(Vex.this.random.nextInt(15) - 7, Vex.this.random.nextInt(11) - 5, Vex.this.random.nextInt(15) - 7); + +- if (Vex.this.level().isEmptyBlock(blockposition1)) { ++ // Paper start - Don't load chunks ++ final net.minecraft.world.level.block.state.BlockState blockState = Vex.this.level().getBlockStateIfLoaded(blockposition1); ++ if (blockState != null && blockState.isAir()) { ++ // Paper end - Don't load chunks + Vex.this.moveControl.setWantedPosition((double) blockposition1.getX() + 0.5D, (double) blockposition1.getY() + 0.5D, (double) blockposition1.getZ() + 0.5D, 0.25D); + if (Vex.this.getTarget() == null) { + Vex.this.getLookControl().setLookAt((double) blockposition1.getX() + 0.5D, (double) blockposition1.getY() + 0.5D, (double) blockposition1.getZ() + 0.5D, 180.0F, 20.0F); diff --git a/patches/server/0978-Validate-ResourceLocation-in-NBT-reading.patch b/patches/server/0978-Validate-ResourceLocation-in-NBT-reading.patch index 2f94b7ad03..eab18ce886 100644 --- a/patches/server/0978-Validate-ResourceLocation-in-NBT-reading.patch +++ b/patches/server/0978-Validate-ResourceLocation-in-NBT-reading.patch @@ -53,7 +53,7 @@ index 8ba573bb4099ee5b27b61f333e72d794c48d5f29..69bdf3f2ee731e59e8d454816a9ca72c @Nullable diff --git a/src/main/java/net/minecraft/world/entity/LivingEntity.java b/src/main/java/net/minecraft/world/entity/LivingEntity.java -index 4591011a142f33f0c0ff84a2765cededde0e0c57..c0bfce7266bbdfe0c5a753367032eb333f56c182 100644 +index 6502dfa62fb0e5a0e3369c62c6ad97aecced4d2b..484056cf77334818aab98ad20e99685d1e240adc 100644 --- a/src/main/java/net/minecraft/world/entity/LivingEntity.java +++ b/src/main/java/net/minecraft/world/entity/LivingEntity.java @@ -871,12 +871,13 @@ public abstract class LivingEntity extends Entity implements Attackable { @@ -110,3 +110,34 @@ index 7529751afa2932fd16bc4591189b0358268a7b14..e2e1c7a017e82dc7299e5cd1783818e4 this.setLootTableSeed(nbt.getLong("LootTableSeed")); } +diff --git a/src/main/java/net/minecraft/world/level/block/entity/AbstractFurnaceBlockEntity.java b/src/main/java/net/minecraft/world/level/block/entity/AbstractFurnaceBlockEntity.java +index d04fc84eef11adb5ea64077f48794b6ed7fb3ada..89d06253b00604114e543ebbe12a9993ae95dc41 100644 +--- a/src/main/java/net/minecraft/world/level/block/entity/AbstractFurnaceBlockEntity.java ++++ b/src/main/java/net/minecraft/world/level/block/entity/AbstractFurnaceBlockEntity.java +@@ -290,7 +290,12 @@ public abstract class AbstractFurnaceBlockEntity extends BaseContainerBlockEntit + while (iterator.hasNext()) { + String s = (String) iterator.next(); + +- this.recipesUsed.put(new ResourceLocation(s), nbttagcompound1.getInt(s)); ++ // Paper start - Validate ResourceLocation ++ final ResourceLocation resourceLocation = ResourceLocation.tryParse(s); ++ if (resourceLocation != null) { ++ this.recipesUsed.put(resourceLocation, nbttagcompound1.getInt(s)); ++ } ++ // Paper end - Validate ResourceLocation + } + + // Paper start - cook speed multiplier API +diff --git a/src/main/java/net/minecraft/world/level/block/entity/BrushableBlockEntity.java b/src/main/java/net/minecraft/world/level/block/entity/BrushableBlockEntity.java +index b5b1831631e233a96b6fd55972a8862b0f420da8..3f62e823bf9b5aa696e3c240613a0fb50340875e 100644 +--- a/src/main/java/net/minecraft/world/level/block/entity/BrushableBlockEntity.java ++++ b/src/main/java/net/minecraft/world/level/block/entity/BrushableBlockEntity.java +@@ -199,7 +199,7 @@ public class BrushableBlockEntity extends BlockEntity { + + private boolean tryLoadLootTable(CompoundTag nbt) { + if (nbt.contains("LootTable", 8)) { +- this.lootTable = new ResourceLocation(nbt.getString("LootTable")); ++ this.lootTable = ResourceLocation.tryParse(nbt.getString("LootTable")); // Paper - Validate ResourceLocation + this.lootTableSeed = nbt.getLong("LootTableSeed"); + return true; + } else { diff --git a/patches/server/1039-Improve-tag-parser-handling.patch b/patches/server/1039-Improve-tag-parser-handling.patch index 2dabc48078..e57cd51b7b 100644 --- a/patches/server/1039-Improve-tag-parser-handling.patch +++ b/patches/server/1039-Improve-tag-parser-handling.patch @@ -5,22 +5,54 @@ Subject: [PATCH] Improve tag parser handling diff --git a/src/main/java/com/mojang/brigadier/CommandDispatcher.java b/src/main/java/com/mojang/brigadier/CommandDispatcher.java -index 92848b64a78fce7a92e1657c2da6fc5ee53eea44..5d0e8f4f3ad61a27452675277380e27d3d28d133 100644 +index 92848b64a78fce7a92e1657c2da6fc5ee53eea44..5d5562676a77259b875e15b744b53258533851a7 100644 --- a/src/main/java/com/mojang/brigadier/CommandDispatcher.java +++ b/src/main/java/com/mojang/brigadier/CommandDispatcher.java -@@ -307,6 +307,10 @@ public class CommandDispatcher { +@@ -304,9 +304,15 @@ public class CommandDispatcher { + } + final CommandContextBuilder context = contextSoFar.copy(); + final StringReader reader = new StringReader(originalReader); ++ boolean stop = false; // Paper - Handle non-recoverable exceptions try { try { child.parse(reader, context); -+ // Paper start - Handle non-reoverable exceptions; Rethrow NbtAccounterException so it can be caught properly and immediately -+ } catch (final net.minecraft.nbt.NbtAccounterException e) { ++ // Paper start - Handle non-recoverable exceptions ++ } catch (final com.mojang.brigadier.exceptions.TagParseCommandSyntaxException e) { ++ stop = true; + throw e; -+ // Paper end - Handle non-reoverable exceptions ++ // Paper end - Handle non-recoverable exceptions } catch (final RuntimeException ex) { throw CommandSyntaxException.BUILT_IN_EXCEPTIONS.dispatcherParseException().createWithContext(reader, ex.getMessage()); } +@@ -321,6 +327,7 @@ public class CommandDispatcher { + } + errors.put(child, ex); + reader.setCursor(cursor); ++ if (stop) return new ParseResults<>(contextSoFar, originalReader, errors); // Paper - Handle non-recoverable exceptions + continue; + } + +diff --git a/src/main/java/com/mojang/brigadier/exceptions/TagParseCommandSyntaxException.java b/src/main/java/com/mojang/brigadier/exceptions/TagParseCommandSyntaxException.java +new file mode 100644 +index 0000000000000000000000000000000000000000..bf248a215dc69bb303c836112309471aab687e23 +--- /dev/null ++++ b/src/main/java/com/mojang/brigadier/exceptions/TagParseCommandSyntaxException.java +@@ -0,0 +1,13 @@ ++package com.mojang.brigadier.exceptions; ++ ++import com.mojang.brigadier.LiteralMessage; ++import net.minecraft.network.chat.Component; ++ ++public final class TagParseCommandSyntaxException extends CommandSyntaxException { ++ ++ private static final SimpleCommandExceptionType EXCEPTION_TYPE = new SimpleCommandExceptionType(new LiteralMessage("Error parsing NBT")); ++ ++ public TagParseCommandSyntaxException(final String message) { ++ super(EXCEPTION_TYPE, Component.literal(message)); ++ } ++} diff --git a/src/main/java/net/minecraft/nbt/TagParser.java b/src/main/java/net/minecraft/nbt/TagParser.java -index 5bec54239a2b185284c10d58854e5a13e33daae5..9ecd0b7ddaa8376f3c1448f810f7757c9ba1b90a 100644 +index 5bec54239a2b185284c10d58854e5a13e33daae5..94cb73e7f60171aa57bd1dbe7e91ef4db94e70b7 100644 --- a/src/main/java/net/minecraft/nbt/TagParser.java +++ b/src/main/java/net/minecraft/nbt/TagParser.java @@ -48,6 +48,7 @@ public class TagParser { @@ -83,10 +115,10 @@ index 5bec54239a2b185284c10d58854e5a13e33daae5..9ecd0b7ddaa8376f3c1448f810f7757c this.reader.expect(c); } + -+ private void increaseDepth() { ++ private void increaseDepth() throws CommandSyntaxException { + this.depth++; + if (this.depth > 512) { -+ throw new net.minecraft.nbt.NbtAccounterException("NBT tag is too complex, depth > 512"); ++ throw new com.mojang.brigadier.exceptions.TagParseCommandSyntaxException("NBT tag is too complex, depth > 512"); + } + } } @@ -104,7 +136,7 @@ index a5e438a834826161c52ca9db57d234d9ff80a591..4766994cce060564370b0d24836a7da8 @Override diff --git a/src/main/java/net/minecraft/server/network/ServerGamePacketListenerImpl.java b/src/main/java/net/minecraft/server/network/ServerGamePacketListenerImpl.java -index 2f9c62f2c4c4356a896f7004b77f12a595f9c6dc..6d943b5c38932de5fc3abb28618bf518c36edb47 100644 +index 2f9c62f2c4c4356a896f7004b77f12a595f9c6dc..ab5b259d8f72022c875cae73be25fe2da346c6b3 100644 --- a/src/main/java/net/minecraft/server/network/ServerGamePacketListenerImpl.java +++ b/src/main/java/net/minecraft/server/network/ServerGamePacketListenerImpl.java @@ -777,6 +777,13 @@ public class ServerGamePacketListenerImpl extends ServerCommonPacketListenerImpl @@ -121,23 +153,17 @@ index 2f9c62f2c4c4356a896f7004b77f12a595f9c6dc..6d943b5c38932de5fc3abb28618bf518 // Paper start - AsyncTabCompleteEvent TAB_COMPLETE_EXECUTOR.execute(() -> this.handleCustomCommandSuggestions0(packet)); } -@@ -823,7 +830,18 @@ public class ServerGamePacketListenerImpl extends ServerCommonPacketListenerImpl - +@@ -824,6 +831,13 @@ public class ServerGamePacketListenerImpl extends ServerCommonPacketListenerImpl private void sendServerSuggestions(final ServerboundCommandSuggestionPacket packet, final StringReader stringreader) { // Paper end - AsyncTabCompleteEvent -- ParseResults parseresults = this.server.getCommands().getDispatcher().parse(stringreader, this.player.createCommandSourceStack()); -+ // Paper start - Handle non-reoverable exceptions -+ ParseResults parseresults; -+ try { -+ parseresults = this.server.getCommands().getDispatcher().parse(stringreader, this.player.createCommandSourceStack()); -+ } catch (final Throwable e) { // This is fine:tm: -+ if (LOGGER.isDebugEnabled()) { -+ LOGGER.error("Exception parsing command", e); -+ } + ParseResults parseresults = this.server.getCommands().getDispatcher().parse(stringreader, this.player.createCommandSourceStack()); ++ // Paper start - Handle non-recoverable exceptions ++ if (!parseresults.getExceptions().isEmpty() ++ && parseresults.getExceptions().values().stream().anyMatch(e -> e instanceof com.mojang.brigadier.exceptions.TagParseCommandSyntaxException)) { + this.disconnect(Component.translatable("disconnect.spam"), org.bukkit.event.player.PlayerKickEvent.Cause.SPAM); + return; + } -+ // Paper end - Handle non-reoverable exceptions ++ // Paper end - Handle non-recoverable exceptions this.server.getCommands().getDispatcher().getCompletionSuggestions(parseresults).thenAccept((suggestions) -> { // Paper start - Don't tab-complete namespaced commands if send-namespaced is false