From 0c7120cdb363c55a5082b28c4dd91a6c9f8f7a71 Mon Sep 17 00:00:00 2001 From: "Joseph C. Sible" Date: Sun, 11 Nov 2018 16:52:03 -0500 Subject: [PATCH] Fix #1865: don't allow packets to trigger NBT changes that wouldn't have been possible to cause via the legitimate GUI --- changelog.txt | 1 + .../screens/NbtSanitizerModuleGuiBuilder.java | 200 ++++++++++++++++++ .../blocks/screens/ScreenTileEntity.java | 7 +- 3 files changed, 206 insertions(+), 2 deletions(-) create mode 100644 src/main/java/mcjty/rftools/blocks/screens/NbtSanitizerModuleGuiBuilder.java diff --git a/changelog.txt b/changelog.txt index 26ce789e5..6cd65c0aa 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,5 +1,6 @@ 7.58: - This release requires that you also upgrade all other mods that add screen modules (RFTools Control, RFTools Dimensions, Deep Resonance) +- Fully fixed the bug where the screen block could let players set arbitrary NBT on items 7.57: - Mitigated a bug where the screen block could let players set arbitrary NBT on items diff --git a/src/main/java/mcjty/rftools/blocks/screens/NbtSanitizerModuleGuiBuilder.java b/src/main/java/mcjty/rftools/blocks/screens/NbtSanitizerModuleGuiBuilder.java new file mode 100644 index 000000000..4fcaf4e9c --- /dev/null +++ b/src/main/java/mcjty/rftools/blocks/screens/NbtSanitizerModuleGuiBuilder.java @@ -0,0 +1,200 @@ +package mcjty.rftools.blocks.screens; + + +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import javax.annotation.Nullable; + +import com.google.common.collect.ImmutableSet; + +import mcjty.rftools.api.screens.FormatStyle; +import mcjty.rftools.api.screens.IModuleGuiBuilder; +import net.minecraft.item.ItemStack; +import net.minecraft.nbt.NBTTagCompound; +import net.minecraft.world.World; + +/** + * Allow only changes to the NBT that could have been legitimately made via the GUI. + */ +public class NbtSanitizerModuleGuiBuilder implements IModuleGuiBuilder { + private final Map> enumKeys = new HashMap<>(); + private final Set stringKeys = new HashSet<>(); + private final Map boundedIntegerKeys = new HashMap<>(); + private final Set integerKeys = new HashSet<>(); + private boolean hasModeKeys = false; + private final Set booleanKeys = new HashSet<>(); + private final Set itemKeys = new HashSet<>(); + private final World world; + @Nullable private final NBTTagCompound oldCompound; + + public NbtSanitizerModuleGuiBuilder(World world, @Nullable NBTTagCompound oldCompound) { + this.world = world; + this.oldCompound = oldCompound; + } + + public NBTTagCompound sanitizeNbt(NBTTagCompound fromClient) { + NBTTagCompound newCompound = oldCompound != null ? oldCompound.copy() : new NBTTagCompound(); + + for(Map.Entry> entry : enumKeys.entrySet()) { + String key = entry.getKey(); + if(fromClient.hasKey(key, 8)) { + String value = fromClient.getString(key); + if(entry.getValue().contains(value)) { + newCompound.setString(key, value); + } + } + } + + for(String key : stringKeys) { + if(fromClient.hasKey(key, 8)) { + newCompound.setString(key, fromClient.getString(key)); + } + } + + for(Map.Entry entry : boundedIntegerKeys.entrySet()) { + String key = entry.getKey(); + if(fromClient.hasKey(key, 3)) { + int value = fromClient.getInteger(key); + if(value >= 0 && value < entry.getValue()) { + newCompound.setInteger(key, value); + } + } + } + + for(String key : integerKeys) { + if(fromClient.hasKey(key, 3)) { + newCompound.setInteger(key, fromClient.getInteger(key)); + } + } + + if(hasModeKeys && fromClient.hasKey("showdiff", 1) && fromClient.hasKey("showpct", 1) && fromClient.hasKey("hidetext", 1)) { + boolean showdiff = fromClient.getBoolean("showdiff"); + boolean showpct = fromClient.getBoolean("showpct"); + boolean hidetext = fromClient.getBoolean("hidetext"); + if(!((showdiff && showpct) || (showdiff && hidetext) || (showpct && hidetext))) { + newCompound.setBoolean("showdiff", showdiff); + newCompound.setBoolean("showpct", showpct); + newCompound.setBoolean("hidetext", hidetext); + } + } + + for(String key : booleanKeys) { + if(fromClient.hasKey(key, 1)) { + newCompound.setBoolean(key, fromClient.getBoolean(key)); + } + } + + for(String key : itemKeys) { + if(fromClient.hasKey(key, 10)) { + NBTTagCompound tag = new NBTTagCompound(); + new ItemStack(fromClient.getCompoundTag(key)).writeToNBT(tag); + newCompound.setTag(key, tag); + } else { + newCompound.removeTag(key); + } + } + + return newCompound; + } + + @Override + public NBTTagCompound getCurrentData() { + return oldCompound.copy(); + } + + @Override + public World getWorld() { + return world; + } + + @Override + public IModuleGuiBuilder choices(String tagname, String tooltip, String... choices) { + enumKeys.put(tagname, ImmutableSet.copyOf(choices)); + return this; + } + + private static final Set FORMAT_STRINGS = ImmutableSet.copyOf(Arrays.stream(FormatStyle.values()).map(FormatStyle::getName).toArray(String[]::new)); + + @Override + public IModuleGuiBuilder format(String tagname) { + enumKeys.put(tagname, FORMAT_STRINGS); + return this; + } + + @Override + public IModuleGuiBuilder text(String tagname, String... tooltip) { + stringKeys.add(tagname); + return this; + } + + @Override + public IModuleGuiBuilder choices(String tagname, Choice... choices) { + boundedIntegerKeys.put(tagname, choices.length); + return this; + } + + @Override + public IModuleGuiBuilder integer(String tagname, String... tooltip) { + integerKeys.add(tagname); + return this; + } + + @Override + public IModuleGuiBuilder color(String tagname, String... tooltip) { + integerKeys.add(tagname); + return this; + } + + @Override + public IModuleGuiBuilder mode(String componentName) { + hasModeKeys = true; + return this; + } + + @Override + public IModuleGuiBuilder toggle(String tagname, String label, String... tooltip) { + booleanKeys.add(tagname); + return this; + } + + @Override + public IModuleGuiBuilder toggleNegative(String tagname, String label, String... tooltip) { + booleanKeys.add(tagname); + return this; + } + + @Override + public IModuleGuiBuilder ghostStack(String tagname) { + itemKeys.add(tagname); + return this; + } + + @Override + public IModuleGuiBuilder label(String text) { + // read-only in GUI + return this; + } + + @Override + public IModuleGuiBuilder leftLabel(String text) { + // read-only in GUI + return this; + } + + @Override + public IModuleGuiBuilder block(String tagname) { + // read-only in GUI + return this; + } + + @Override + public IModuleGuiBuilder nl() { + // read-only in GUI + return this; + } + +} diff --git a/src/main/java/mcjty/rftools/blocks/screens/ScreenTileEntity.java b/src/main/java/mcjty/rftools/blocks/screens/ScreenTileEntity.java index 721409943..7c5590a97 100644 --- a/src/main/java/mcjty/rftools/blocks/screens/ScreenTileEntity.java +++ b/src/main/java/mcjty/rftools/blocks/screens/ScreenTileEntity.java @@ -570,11 +570,14 @@ public boolean isConnected() { public void updateModuleData(int slot, NBTTagCompound tagCompound) { ItemStack stack = inventoryHelper.getStackInSlot(slot); - if(stack.isEmpty() || !ScreenBlock.hasModuleProvider(stack)) { + IModuleProvider moduleProvider = ScreenBlock.getModuleProvider(stack); + if(stack.isEmpty() || moduleProvider == null) { Logging.logError("PacketModuleUpdate: ItemStack does not have a module provider!"); return; } - stack.setTagCompound(tagCompound); + NbtSanitizerModuleGuiBuilder sanitizer = new NbtSanitizerModuleGuiBuilder(getWorld(), stack.getTagCompound()); + moduleProvider.createGui(sanitizer); + stack.setTagCompound(sanitizer.sanitizeNbt(tagCompound)); screenModules = null; clientScreenModules = null; computerModules.clear();