Skip to content
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

ForceOP AntiMalware #438

Closed
Link0Darck opened this issue Nov 21, 2021 · 5 comments
Closed

ForceOP AntiMalware #438

Link0Darck opened this issue Nov 21, 2021 · 5 comments
Assignees

Comments

@Link0Darck
Copy link

Good morning,
the AntiMalware has detected a force op command in your plugin which is vulnerable if you don't check by yourself with the spigot staff plugin
plugin : (https://www.spigotmc.org/resources/spigot-anti-malware.64982/)
[AntiMalware] [01:10:08] [INFO]: Using locale en [AntiMalware] [01:10:08] [INFO]: Initializing [AntiMalware] [01:10:08] [INFO]: Any bugs and/or false-positives should be reported either on the GitHub repo, plugin discussion page, or on the discord server [AntiMalware] [01:10:08] [INFO]: Registering checks [AntiMalware] [01:10:08] [INFO]: Finished registering checks [AntiMalware] [01:10:08] [INFO]: Setting up the Auto-Updater [AntiMalware] [01:10:08] [INFO]: Finished initializing [AntiMalware] [01:10:08] [DETECTED]: plugins\ItemJoin.jar MIGHT be infected with Spigot.MALWARE.ForceOP.A Class Path: me/RockinChaos/itemjoin/item/ItemCommand ; SourceFile/Line ItemCommand.java/351

@RockinChaos
Copy link
Owner

RockinChaos commented Nov 21, 2021

I wouldn't say its malware but force op does exist in ItemJoin.
Link can be found here;

It exists for the op: command identifier so Item Commands can be executed as an elevated (op) player. If no item contains the op: identifier command then it will never be used and is impossible to be used unless that is defined.

In addition, the force op code as shown in the link about is a completely closed loop so it doesn't allow anyone with malicious intent to execute any commands while they are op and if any errors occur it automatically removes them from OP in the catch clause. They are only op for a roughly a millisecond, the time it takes to dispatch the command, then they are automatically returned to their original OP state (either non-op) or if they were already OP such as an admin they will remain OP.

This code has existed in its current state for about 6 years without any reports of issues, also other plugins are using pretty much the exact same "closed-loop" code and is known to be the safest way to do so. ItemJoin does have several warnings when trying to use the op: command identifier to do so at your own discretion.

Hopefully that makes sense, essentially if you don't want to have players ever be forced op don't use the op: command identifier for Item Commands.

If you need any additional clarification or anything else let me know, I am happy to help out where I can!

EDIT: I am currently still looking for it but I did ask on the spigot forums a long time ago about any concerns regarding security issues with the closed-loop code as shown in the link above and MD_5 actually commented on it and said that is the safest way to do so. Soon as I find the link I will include it as a reference.

@RockinChaos
Copy link
Owner

RockinChaos commented Nov 21, 2021

Ah okay so I see why it flagged it as malware now, I checked the source code of the anti-malware plugin since it is open source, if I had it specify specifically player.setOp(false) directly after setting them to player.setOp(true) it wouldn't have flagged it. The reason its flagged is because I use a dynamic variable, I fetch the players OP status prior to setting them to OP then I set them back to that status after they are set to OP. If I didn't do that then some players who were already OP prior to executing the command would end up getting de-opped when they are not supposed to.

Essentially there is no way for the malware plugin to know what the dynamic variable is since it can technically be true or false.
If that makes sense.

Considering this new knowledge I think I can add a catch to ignore OP players and manually set to false, so stay tuned for the next snapshot as it will resolve this error since I will use a static variable.

I will reply with a snapshot once it is compiled.

@RockinChaos
Copy link
Owner

Okay so, I have fixed the issue stated above so MCAntiMalware no longer throws false positives, I use static variables now that it can clearly see. I ran MCAntiMalware and ItemJoin now passes with no reports.

Link; https://ci.craftationgaming.com/job/ItemJoin/779/
Remember to remove your old ItemJoin.jar file

Please see the devlog for the current changes since the last release; https://github.com/RockinChaos/ItemJoin/wiki/DevLog

Let me know!~

@RockinChaos RockinChaos added Fixed - Pending Release Pending official release, currently is fixed in the latest snapshot. and removed Bug Testing labels Nov 21, 2021
@RockinChaos
Copy link
Owner

Officially fixed in the latest release,

You can now download it here; https://www.spigotmc.org/resources/itemjoin.12661/download?version=431799

@RockinChaos RockinChaos added Resolved and removed Fixed - Pending Release Pending official release, currently is fixed in the latest snapshot. labels Dec 16, 2021
@Link0Darck
Copy link
Author

I'm sorry I had some unforeseen events in my life, but you solved the problem. Thank you and I saw that it is compatible with the 1.8.8

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

No branches or pull requests

2 participants