Unsecure Packets
Sir-Will opened this issue ยท 11 comments
Hey,
The packets in this mod are very unsecure, any mod can send the AddItem or Craft packet to cheat items on servers. This is being abused in hacks.
Instead of sending the ItemStack to the server you should send the slot id's to the server and handle the crafting on server side.
This is true, do you have records of this happening? In the back of my head I knew this issue could arrive but I doubted it would happen.
I think the AddItem packet could actually be removed entirely, as I don't think I use it anymore.
The issue with the Craft packet is that the held
ItemStack (the stack which is held under the mouse in a GUI) is only available on the client (for obvious reasons). So I must request the ItemStack as a Packet parameter. The slot check is still done on the server though, but the hacker could theoretically call the craft packet while having only one part of a recipe and "emulating" the other part. Think of having sticks in the slot but telling the server that you have diamonds under the mouse to create a diamond sword from basically nothing. Please let me know if you have any suggestions on how to fix this.
The exploit is being used and shared here: https://www.youtube.com/watch?v=mlMltNgzQ5E
I would say to use an inventory click event but there doesn't seem to be one for forge.
Otherwise, you could send the ItemStack the player is holding and then check on the server side if the player has that ItemStack in his inventory and only add the crafting result if you could successfully remove the required ItemStacks from the players inventory.
That's just what he tells people as excuse, he even took the time to obfuscate the jar to make it more difficult for others to find what mod/packet is being used.
Problem with your suggested fix is that the player then actually doesn't have the stack in the inventory anymore. The held stack is invisible to the server entirely.
Are you sure? I always thought the ItemStack is still on that specific slot until it's placed somewhere else, for the server.
You might need to ask on the forge forums then. I don't know a lot about forge.
Bukkit and Sponge can get the cursor ItemStack so there must be a way to do that in forge.
Also, shouldn't InventoryPlayer#getItemStack()
have the @SideOnly(Side.CLIENT)
annotation if it's not available on the server?
I will step through the debugger and check closely. I'm already talking to some people on how to get this fixed. The AddItem Packet was removed.
While the method is also available on the server, it always returns ItemStack.EMPTY because it's never set (I think, but I'll test now)
And yeah I went trough his code and it's a right mess. Clearly done with malicious intent...
So I investigated some more, it seems the held stack is available on the server after all. I'll have a fix out momentarily.
Wow, I might have underestimated the problem a bit. It works on singleplayer now, but doesn't work at all on servers. Seems the held ItemStack doesn't sync to servers but does sync if you're in a singleplayer world.
"Sharing exploits so people can fix them" Yeah sure buddy, then why haven't I seen this sooner? It's a month old already. I assume you aren't the original poster of the video. The author never tried to contact me in any way.
Problem with your suggested fix is that the player then actually doesn't have the stack in the inventory anymore. The held stack is invisible to the server entirely.