papermc/Paper
0
0
Fork 0
mirror of https://github.com/PaperMC/Paper.git synced 2024-11-22 04:56:29 +00:00
Code Issues
master
Paper/patches/server/0909-Validate-ResourceLocation-in-NBT-reading.patch
Spottedleaf 8c5b837e05 Rework async chunk api implementation 
Firstly, the old methods all routed to the CompletableFuture method.
However, the CF method could not guarantee that if the caller
was off-main that the future would be "completed" on-main. Since
the callback methods used the CF one, this meant that the callback
methods did not guarantee that the callbacks were to be called on
the main thread.

Now, all methods route to getChunkAtAsync(x, z, gen, urgent, cb)
so that the methods with the callback are guaranteed to invoke
the callback on the main thread. The CF behavior remains unchanged;
it may still appear to complete on main if invoked off-main.

Secondly, remove the scheduleOnMain invocation in the async
chunk completion. This unnecessarily delays the callback
by 1 tick.

Thirdly, add getChunksAtAsync(minX, minZ, maxX, maxZ, ...) which
will load chunks within an area. This method is provided as a helper
as keeping all chunks loaded within an area can be complicated to
implement for plugins (due to the lacking ticket API), and is
already implemented internally anyways.

Fourthly, remove the ticket addition that occured with getChunkAt
and getChunkAtAsync. The ticket addition may delay the unloading
of the chunk unnecessarily. It also fixes a very rare timing bug
where the future/callback would be completed after the chunk
unloads.
2024-11-18 23:00:59 -08:00

174 lines
12 KiB
Diff
Raw Permalink Blame History

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Nassim Jahnke <nassim@njahnke.dev>
Date: Thu, 4 Jan 2024 13:49:14 +0100
Subject: [PATCH] Validate ResourceLocation in NBT reading
diff --git a/src/main/java/net/minecraft/nbt/NbtUtils.java b/src/main/java/net/minecraft/nbt/NbtUtils.java
index 4929bac8e476664086470f078efce6c0a6164413..f88dd37783b3c155c23b547c360b8d3c16e030c0 100644
--- a/src/main/java/net/minecraft/nbt/NbtUtils.java
+++ b/src/main/java/net/minecraft/nbt/NbtUtils.java
@@ -149,8 +149,10 @@ public final class NbtUtils {
if (!nbt.contains("Name", 8)) {
return Blocks.AIR.defaultBlockState();
} else {
- ResourceLocation resourceLocation = ResourceLocation.parse(nbt.getString("Name"));
- Optional<? extends Holder<Block>> optional = blockLookup.get(ResourceKey.create(Registries.BLOCK, resourceLocation));
+ // Paper start - Validate resource location
+ ResourceLocation resourceLocation = ResourceLocation.tryParse(nbt.getString("Name"));
+ Optional<? extends Holder<Block>> optional = resourceLocation != null ? blockLookup.get(ResourceKey.create(Registries.BLOCK, resourceLocation)) : Optional.empty();
+ // Paper end - Validate resource location
if (optional.isEmpty()) {
return Blocks.AIR.defaultBlockState();
} else {
diff --git a/src/main/java/net/minecraft/resources/ResourceLocation.java b/src/main/java/net/minecraft/resources/ResourceLocation.java
index 87afe84791af2d5e9f869cd4c09eed4bb5fee75b..1967c43ee3a12e63365cc40ee6565307e2fd73cf 100644
--- a/src/main/java/net/minecraft/resources/ResourceLocation.java
+++ b/src/main/java/net/minecraft/resources/ResourceLocation.java
@@ -41,6 +41,13 @@ public final class ResourceLocation implements Comparable<ResourceLocation> {
assert isValidPath(path);
+ // Paper start - Validate ResourceLocation
+ // Check for the max network string length (capped at Short.MAX_VALUE) as well as the max bytes of a StringTag (length written as an unsigned short)
+ final String resourceLocation = namespace + ":" + path;
+ if (resourceLocation.length() > Short.MAX_VALUE || io.netty.buffer.ByteBufUtil.utf8MaxBytes(resourceLocation) > 2 * Short.MAX_VALUE + 1) {
+ throw new ResourceLocationException("Resource location too long: " + resourceLocation);
+ }
+ // Paper end - Validate ResourceLocation
this.namespace = namespace;
this.path = path;
}
diff --git a/src/main/java/net/minecraft/world/RandomizableContainer.java b/src/main/java/net/minecraft/world/RandomizableContainer.java
index 084935138b1484f3d96e99f4e5655a6c04931907..9e357abe13f55bd9ce3a1d5348bcf19a15ea5433 100644
--- a/src/main/java/net/minecraft/world/RandomizableContainer.java
+++ b/src/main/java/net/minecraft/world/RandomizableContainer.java
@@ -50,7 +50,7 @@ public interface RandomizableContainer extends Container {
default boolean tryLoadLootTable(CompoundTag nbt) {
if (nbt.contains("LootTable", 8)) {
- this.setLootTable(ResourceKey.create(Registries.LOOT_TABLE, ResourceLocation.parse(nbt.getString("LootTable"))));
+ this.setLootTable(net.minecraft.Optionull.map(ResourceLocation.tryParse(nbt.getString("LootTable")), rl -> ResourceKey.create(Registries.LOOT_TABLE, rl))); // Paper - Validate ResourceLocation
if (this.lootableData() != null && this.getLootTable() != null) this.lootableData().loadNbt(nbt); // Paper - LootTable API
if (nbt.contains("LootTableSeed", 4)) {
this.setLootTableSeed(nbt.getLong("LootTableSeed"));
diff --git a/src/main/java/net/minecraft/world/entity/EntityType.java b/src/main/java/net/minecraft/world/entity/EntityType.java
index c2693d530be00af16b2aa4ca4afd1d136db68183..629c1316920ad4c111fff489f8c3ea0ed39d0099 100644
--- a/src/main/java/net/minecraft/world/entity/EntityType.java
+++ b/src/main/java/net/minecraft/world/entity/EntityType.java
@@ -680,7 +680,7 @@ public class EntityType<T extends Entity> implements FeatureElement, EntityTypeT
}
public static Optional<EntityType<?>> by(CompoundTag nbt) {
- return BuiltInRegistries.ENTITY_TYPE.getOptional(ResourceLocation.parse(nbt.getString("id")));
+ return BuiltInRegistries.ENTITY_TYPE.getOptional(ResourceLocation.tryParse(nbt.getString("id"))); // Paper - Validate ResourceLocation
}
@Nullable
diff --git a/src/main/java/net/minecraft/world/entity/Leashable.java b/src/main/java/net/minecraft/world/entity/Leashable.java
index b7721ed97305d1cd6725935f965c2effc1bef5a1..5f880a8809f9c20bc8e8c0b2d48590bab02cf077 100644
--- a/src/main/java/net/minecraft/world/entity/Leashable.java
+++ b/src/main/java/net/minecraft/world/entity/Leashable.java
@@ -55,7 +55,13 @@ public interface Leashable {
@Nullable
default Leashable.LeashData readLeashData(CompoundTag nbt) {
if (nbt.contains("leash", 10)) {
- return new Leashable.LeashData(Either.left(nbt.getCompound("leash").getUUID("UUID")));
+ // Paper start
+ final CompoundTag leashTag = nbt.getCompound("leash");
+ if (!leashTag.hasUUID("UUID")) {
+ return null;
+ }
+ return new Leashable.LeashData(Either.left(leashTag.getUUID("UUID")));
+ // Paper end
} else {
if (nbt.contains("leash", 11)) {
Either<UUID, BlockPos> either = (Either) NbtUtils.readBlockPos(nbt, "leash").map(Either::right).orElse(null); // CraftBukkit - decompile error
diff --git a/src/main/java/net/minecraft/world/entity/LivingEntity.java b/src/main/java/net/minecraft/world/entity/LivingEntity.java
index bd2df2a70e78b68b5eefd9c0b6fe14f9717fb1d7..c1c92bdbe133bd60cf2920a2195bbb15d0f94455 100644
--- a/src/main/java/net/minecraft/world/entity/LivingEntity.java
+++ b/src/main/java/net/minecraft/world/entity/LivingEntity.java
@@ -909,11 +909,13 @@ public abstract class LivingEntity extends Entity implements Attackable {
if (nbt.contains("SleepingX", 99) && nbt.contains("SleepingY", 99) && nbt.contains("SleepingZ", 99)) {
BlockPos blockposition = new BlockPos(nbt.getInt("SleepingX"), nbt.getInt("SleepingY"), nbt.getInt("SleepingZ"));
+ if (this.position().distanceToSqr(blockposition.getX(), blockposition.getY(), blockposition.getZ()) < 16 * 16) { // Paper - The sleeping pos will always also set the actual pos, so a desync suggests something is wrong
this.setSleepingPos(blockposition);
this.entityData.set(LivingEntity.DATA_POSE, Pose.SLEEPING);
if (!this.firstTick) {
this.setPosToBed(blockposition);
}
+ } // Paper - The sleeping pos will always also set the actual pos, so a desync suggests something is wrong
}
if (nbt.contains("Brain", 10)) {
diff --git a/src/main/java/net/minecraft/world/entity/Mob.java b/src/main/java/net/minecraft/world/entity/Mob.java
index 8a0e65ac8318a467996f48b423db1ac621359fbe..aad63549d7c4f501b683b8dead4938eac27895eb 100644
--- a/src/main/java/net/minecraft/world/entity/Mob.java
+++ b/src/main/java/net/minecraft/world/entity/Mob.java
@@ -601,7 +601,7 @@ public abstract class Mob extends LivingEntity implements EquipmentUser, Leashab
this.leashData = this.readLeashData(nbt);
this.setLeftHanded(nbt.getBoolean("LeftHanded"));
if (nbt.contains("DeathLootTable", 8)) {
- this.lootTable = Optional.of(ResourceKey.create(Registries.LOOT_TABLE, ResourceLocation.parse(nbt.getString("DeathLootTable"))));
+ this.lootTable = Optional.ofNullable(ResourceLocation.tryParse(nbt.getString("DeathLootTable"))).map((rs) -> ResourceKey.create(Registries.LOOT_TABLE, rs)); // Paper - Validate ResourceLocation
this.lootTableSeed = nbt.getLong("DeathLootTableSeed");
}
diff --git a/src/main/java/net/minecraft/world/entity/projectile/AbstractArrow.java b/src/main/java/net/minecraft/world/entity/projectile/AbstractArrow.java
index 75cc3db39c974abab8510af4a633fc6812efc647..14e31ae88e90d8ea1a98800cc6c1c3527bb2ed6b 100644
--- a/src/main/java/net/minecraft/world/entity/projectile/AbstractArrow.java
+++ b/src/main/java/net/minecraft/world/entity/projectile/AbstractArrow.java
@@ -695,7 +695,7 @@ public abstract class AbstractArrow extends Projectile {
this.setCritArrow(nbt.getBoolean("crit"));
this.setPierceLevel(nbt.getByte("PierceLevel"));
if (nbt.contains("SoundEvent", 8)) {
- this.soundEvent = (SoundEvent) BuiltInRegistries.SOUND_EVENT.getOptional(ResourceLocation.parse(nbt.getString("SoundEvent"))).orElse(this.getDefaultHitGroundSoundEvent());
+ this.soundEvent = (SoundEvent) BuiltInRegistries.SOUND_EVENT.getOptional(ResourceLocation.tryParse(nbt.getString("SoundEvent"))).orElse(this.getDefaultHitGroundSoundEvent()); // Paper - Validate resource location
}
if (nbt.contains("item", 10)) {
diff --git a/src/main/java/net/minecraft/world/entity/vehicle/ContainerEntity.java b/src/main/java/net/minecraft/world/entity/vehicle/ContainerEntity.java
index 874a44ab77248665c2db243764e8542bfc0d6514..cc7826a10f22e3307231d887db2fee98063b1f46 100644
--- a/src/main/java/net/minecraft/world/entity/vehicle/ContainerEntity.java
+++ b/src/main/java/net/minecraft/world/entity/vehicle/ContainerEntity.java
@@ -73,7 +73,7 @@ public interface ContainerEntity extends Container, MenuProvider {
default void readChestVehicleSaveData(CompoundTag nbt, HolderLookup.Provider registries) {
this.clearItemStacks();
if (nbt.contains("LootTable", 8)) {
- this.setContainerLootTable(ResourceKey.create(Registries.LOOT_TABLE, ResourceLocation.parse(nbt.getString("LootTable"))));
+ this.setContainerLootTable(net.minecraft.Optionull.map(ResourceLocation.tryParse(nbt.getString("LootTable")), rl -> ResourceKey.create(Registries.LOOT_TABLE, rl))); // Paper - Validate ResourceLocation
// Paper start - LootTable API
if (this.getContainerLootTable() != null) {
this.lootableData().loadNbt(nbt);
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 b1067a3add5dc0cfa853b02b5b556d6d67e2932a..15e0861486a2bda3e2f4049b1b5a299c870acd31 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
@@ -180,7 +180,11 @@ public abstract class AbstractFurnaceBlockEntity extends BaseContainerBlockEntit
while (iterator.hasNext()) {
String s = (String) iterator.next();
- this.recipesUsed.put(ResourceKey.create(Registries.RECIPE, ResourceLocation.parse(s)), nbttagcompound1.getInt(s));
+ // Paper start - Validate ResourceLocation
+ final ResourceLocation resourceLocation = ResourceLocation.tryParse(s);
+ if (resourceLocation != null) {
+ this.recipesUsed.put(ResourceKey.create(Registries.RECIPE, resourceLocation), nbttagcompound1.getInt(s));
+ }
}
// 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 1bfffbf54b1b440c6e19a908ea2bd70387d06b5c..b08867878e56f88569d547765f29cab018a9e791 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
@@ -194,7 +194,7 @@ public class BrushableBlockEntity extends BlockEntity {
private boolean tryLoadLootTable(CompoundTag nbt) {
if (nbt.contains("LootTable", 8)) {
- this.lootTable = ResourceKey.create(Registries.LOOT_TABLE, ResourceLocation.parse(nbt.getString("LootTable")));
+ this.lootTable = net.minecraft.Optionull.map(ResourceLocation.tryParse(nbt.getString("LootTable")), rl -> ResourceKey.create(Registries.LOOT_TABLE, rl)); // Paper - Validate ResourceLocation
this.lootTableSeed = nbt.getLong("LootTableSeed");
return true;
} else {
View Git Blame Copy Permalink