Project Red - Fabrication

Project Red - Fabrication

19M Downloads

[Hacking] ChipNBTSet packet

TheAndrey opened this issue ยท 10 comments

commented

I found unsafe packet handler.
https://github.com/MrTJP/ProjectRed/blob/master/src/mrtjp/projectred/transportation/packethandlers.scala#L178-L179

    private def setChipNBT(packet:PacketCustom, player:EntityPlayerMP)
    {
        val slot = packet.readUByte()
        val stack = packet.readItemStack()
        player.inventory.setInventorySlotContents(slot, stack)
        player.inventory.markDirty()
}

This puts received from client item to player inventory. Hackers can use this to get ANY item on the server!
You can never trust data that you receive from a client. You should always check what you have received from the client.
In this case, you only need to transfer the NBT tag, but not the entire item.

commented

@TheAndrey, I can say that developer doesn`t think about servers, because I finded this bug an year ago, but then I join into discord i was kicked.

commented

ProjectRed doesn't have a discord, I've added this to my list and will check it out this weekend.

commented

This is becoming a serious issue on servers.

commented

Agreed. I patched it manually because we couldn't wait for a patch.

commented

Since i believe there won't be a patch soon and i don't have that good of a knowledge of how to fix it could you tell me how you did it?

commented

I apparently might have a fix just have no idea of how to use it

The current code
https://github.com/MrTJP/ProjectRed/blob/master/src/mrtjp/projectred/transportation/packethandlers.scala#L178-L179
GitHub
MrTJP/ProjectRed
ProjectRed - Extending redstone functionality to FMP

Lines 178 and 179
It can be fixed by replacing line's 175 below with this code
private def setChipNBT(packet:PacketCustom, player:EntityPlayerMP)
    {
        val slot = packet.readUByte()
        val stack = packet.readItemStack()
        val playerstack = player.inventory.getStackInSlot(slot)
        if (stack.getUnlocalizedName == playerstack.getUnlocalizedName)
        {
            player.inventory.setInventorySlotContents(slot, stack)
            player.inventory.markDirty()
        }
        else
        {
            player.addChatMessage(new ChatComponentText("YOU DIRTY HACKER"))
        }
    }
commented

@man-of-stone50 this fix is not safe. Hackers can write any NBT tag to item (like item name, lore, enchantments). Also using getUnlocalizedName() is not good idea, use getItem()

commented

Oh damn that sounds bad, we might have to remove ProjectRed Transportation temporarily.

commented

@MrTJP Will the exploit be fixed?

commented

Fixed via 121cca1 and a0c65c4.