InventoryDragEvent is not handled/cancelled in InventoryListener

[Phoenix616]

Currently the InventoryDragEvent is not handled in the InventoryListener when using this in a bukkit plugin. Seeing as dragging is triggered just by moving your mouse 1px while clicking down on an item this could lead to entries being apparently unresponsive if not being very careful. (And potential other issues e.g. ability to drag items in/out of inventories)

EDIT: Just took a look at the Bungee/Velocity implementation as well. They too seem to only react on the click, not drag (although I'm not sure if Protocolize's Inventory#onClick method maybe gets called for the dragging packet too?)

[KotlinFactory]

Hey,

thanks for using Cirrus & your help. Dragging is not handled by Protocolize at all - So handling it won't be possible without altering it.

Looking on the Spigot side: Currently, we are not handling it here either. In the time we used Cirrus in production it did not cause any issue so far. However, we look to further improve Cirrus. Did I understand correctly that you suggest canceling the InventoryDragEvent? We are also open to PR.

Regards, Leonhard

[Phoenix616]

I'm not really using it, I have my own library for that at least on the Bukkit side and hence am just always interested in how others handle similar cases. (And adding them to the spigot wiki page on inventory GUIs)

Not cancelling/handling the drag event is unfortunately very common as most people don't realise that it's not the same as a click or how easy it is to happen accidentally and that it opens up potential exploits in plugins so I tend to point it out to everyone with such a library that doesn't properly handle this case so that they and their users don't fall prey to such exploits.

[KotlinFactory]

Alright as it did not cause any issue in ~3 years of usage even on servers w/800 current players we will just leave it as it is right now if there is no PR with some reproducible issue.

[Phoenix616]

Well I just hope nobody else is using this library for anything important because this is pretty irresponsible... duping exploits aren't a non-issue and you will only realise that it's being abused when it's too late.

[KotlinFactory]

You were unable to explain any sort of "exploits" when I asked you to do so. The only "exploit" you elaborated on was that the menu could become unresponsive. We checked this and no, dragging won't make it possible to move items to the inventory of players. The code behind this has been in use for ~3 years and there was no problem so far. If you have any improvement to make just create a PR. Btw: If you really think that this would allow for "duping exploits" it would be irresponsible from your side to not open up a PR / further explain your position.

[Phoenix616]

I am not responsible for the security of your software/services. Of course if you can provide me with a link regarding how you/your company handles responsible disclosure and your vulnerability reward program I am absolutely willing to build a working exploit for you if that's really what you want me to do.

I just feel like it would be a lot faster (even than if I would have to familiarise myself with your project in order to open a PR) if you added a listener that does what you want in the context of your project and be done with it...

[KotlinFactory]

As you might have been able to see we already added this listener as we don't want to take any chance with security issues.

I never said you were responsible for our security. See We obviously don't have a vulnerability reward program. And if the issue is that common why would it take a reward program to come up with a vulnerability? It would have been great if you would have been able to provide anything specific than the vague explanations you made.

Anyhow, thanks for your help. In the future, we have to find a better way of communicating as I feel like we wasted a lot of time over nothing here.