New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Hacking] ChipNBTSet packet #1384

Open
TheAndrey opened this Issue Aug 12, 2018 · 8 comments

Comments

Projects
None yet
5 participants
@TheAndrey

TheAndrey commented Aug 12, 2018

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.

@TheAndrey TheAndrey changed the title from ChipNBTSet packet issue to [Hacking] ChipNBTSet packet Aug 14, 2018

@Zaxar163

This comment has been minimized.

Show comment
Hide comment
@Zaxar163

Zaxar163 Aug 16, 2018

@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.

Zaxar163 commented Aug 16, 2018

@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.

@covers1624

This comment has been minimized.

Show comment
Hide comment
@covers1624

covers1624 Aug 16, 2018

Collaborator

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

Collaborator

covers1624 commented Aug 16, 2018

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

@man-of-stone50

This comment has been minimized.

Show comment
Hide comment
@man-of-stone50

man-of-stone50 Aug 28, 2018

This is becoming a serious issue on servers.

man-of-stone50 commented Aug 28, 2018

This is becoming a serious issue on servers.

@FoxMcloud5655

This comment has been minimized.

Show comment
Hide comment
@FoxMcloud5655

FoxMcloud5655 Aug 28, 2018

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

FoxMcloud5655 commented Aug 28, 2018

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

@man-of-stone50

This comment has been minimized.

Show comment
Hide comment
@man-of-stone50

man-of-stone50 Aug 28, 2018

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?

man-of-stone50 commented Aug 28, 2018

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?

@man-of-stone50

This comment has been minimized.

Show comment
Hide comment
@man-of-stone50

man-of-stone50 Aug 28, 2018

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"))
        }
    }

man-of-stone50 commented Aug 28, 2018

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"))
        }
    }
@TheAndrey

This comment has been minimized.

Show comment
Hide comment
@TheAndrey

TheAndrey Aug 28, 2018

@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()

TheAndrey commented Aug 28, 2018

@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()

@man-of-stone50

This comment has been minimized.

Show comment
Hide comment
@man-of-stone50

man-of-stone50 Aug 28, 2018

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

man-of-stone50 commented Aug 28, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment