Trivial item duplication exploit in InvUI caused by `BaseWindow#close`

[Sculas]

I've made a voting plugin that uses InvUI, but after the GUI is closed users can duplicate items. I'm able to reproduce this issue on Paper 1.18.2 and 1.19.2 respectively.

Below is a Paper plugin I made for this issue that can reproduce the exploit.

Plugin JAR: itemdup-1.0-all.jar Plugin source: invui-itemdup-exploit

Reproduction steps:

• 1. Download/Build and install the plugin on a Paper 1.18/1.19 server like usual (this plugin uses Paper API)
• 2. Log into the server
• 3. Observe that the exploit does not work
• 4. Type opengui in chat, and wait for the GUI to close after 3 seconds
• 5. Observe that the exploit does work.

Additional:

• 6. Leave the server
• 7. Observe that the exploit no longer works
• 8. Follow step 4
• 9. Observe that the exploit works again like in step 5

Before a GUI is opened:

https://user-images.githubusercontent.com/22832313/192151479-0ec300af-43b5-409b-b383-1462d54aa03f.mp4

As you can see, everything works as usual here. There's no item duplication exploit. Now, let's run /vote which will open a GUI:

After a GUI is opened:

https://user-images.githubusercontent.com/22832313/192151504-a8f27900-ef32-44a6-8a46-aca0f5560612.mp4

What is happening here?! There's a very bad item duplication exploit that is trivial to abuse. I'm unable to use this plugin on my server at the moment, because of this exploit.

One important note: I'm able to reproduce this issue consistently when shift clicking and dragging as shown in the video above. The reason why this happens still eludes me. I'm using InvUI version 0.8-SNAPSHOT: de.studiocode.invui:InvUI:0.8-SNAPSHOT.

Actually, while writing the test plugin (the block above was written before), I found out that this issue is caused by window.close. If you close the UI yourself, the exploit doesn't occur. But only when window.close is called, does it occur. When you leave, the exploit is gone. You have to open the GUI again, and then the exploit is back.

https://github.com/Sculas/invui-itemdup-exploit/blob/eb4dcd37f013d946ea42d6eb90e9bc87ef589455/src/main/java/xyz/sculas/itemdup/ItemDup.java#L31-L36

[Sculas]

Doing window.close(false) still causes the exploit. So it must be in the body of BaseWindow#close.

Interestingly enough, while testing I also found another bug. If you do window.close(false), close the GUI yourself and then leave, this exception will be thrown:

[18:54:59 WARN]: [ItemDup] Task #25 for ItemDup v1.0 generated an exception
java.lang.IllegalStateException: The Window has already been closed.
        at de.studiocode.invui.window.impl.BaseWindow.show(BaseWindow.java:249) ~[itemdup-1.0-all.jar:?]
        at org.bukkit.craftbukkit.v1_19_R1.scheduler.CraftTask.run(CraftTask.java:101) ~[paper-1.19.2.jar:git-Paper-160]
        at org.bukkit.craftbukkit.v1_19_R1.scheduler.CraftScheduler.mainThreadHeartbeat(CraftScheduler.java:483) ~[paper-1.19.2.jar:git-Paper-160]
        at net.minecraft.server.MinecraftServer.tickChildren(MinecraftServer.java:1493) ~[paper-1.19.2.jar:git-Paper-160]
        at net.minecraft.server.dedicated.DedicatedServer.tickChildren(DedicatedServer.java:446) ~[paper-1.19.2.jar:git-Paper-160]
        at net.minecraft.server.MinecraftServer.tickServer(MinecraftServer.java:1417) ~[paper-1.19.2.jar:git-Paper-160]
        at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1193) ~[paper-1.19.2.jar:git-Paper-160]
        at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:305) ~[paper-1.19.2.jar:git-Paper-160]
        at java.lang.Thread.run(Thread.java:833) ~[?:?]

This is a different issue, but this is caused by BaseWindow#handleClose being called when an inventory is closed (which happens when a player leaves) while it's not closeable (which is why it's being reopened and throws this exception).

[NichtStudioCode]

Should be fixed in c315c360a2022ea95f525cbd4d7d4af3b381a8c0.

Interestingly enough, while testing I also found another bug. If you do window.close(false), close the GUI yourself and then leave, this exception will be thrown

This should also be fixed now. Though, you should never call window.close(false) (it unregisters all handlers but leaves the inventory open -> players can just take the items from the inventory). It only exists for internal use and shouldn't even be accessible to you, the same goes for all the handle... methods. I just didn't have the time to clean that stuff up yet.

[Sculas]

I don't use window.close(false) myself, it was just something I accidentally stumbled upon while testing if maybe that could be it. But thanks a lot for fixing it! :)

[Sculas]

Can the version be bumped? I'll use JitPack for now, but I really think this fix is worth a version bump. Actually: the JitPack build fails and outputs incomplete artifacts.

[NichtStudioCode]

sure

[Sculas]

Thanks!

Just tested the latest version and can confirm that it is now fixed. Thanks a lot! :)