PSA: Magix Plugin

Discussion in 'Community News and Announcements' started by Jadedcat, Oct 21, 2014.

Thread Status:
Not open for further replies.
  1. Offline

    lol768

    I imagine this wasn't done by hand :p
     
  2. Offline

    FerusGrim

    God, I hoped not. But with the crazy, obsessive dedication I'm sure we've all seen from people on the web, I couldn't be sure.
     
  3. Offline

    MrAwellstein


    Its actually really simple to convert it into byte code, and then into JS. Most of the JS was in fact just ByteArrayOutputStream.write(<int>); and that can be achieved by a simple loop (below) so I cant imagine it took very long to create.
    Show Spoiler

    Code:java
    1. for(byte b: bytes){
    2. System.out.println("ByteArrayOutputStream.write("+b+");");
    3. }


    I'm not much for JS, but I used(using) something similar in one of my projects, which allows for remote code execution (so for when im bored and want to do random things to my server at home while im at school). Basically it was a custom ClassLoader which turned a byte[] into a Class which then I could execute.
     
  4. Offline

    FerusGrim

    MrAwellstein
    I was referring to if he had done the byte conversion by hand.
     
  5. Offline

    MrAwellstein


    Oh... Wait is that even possible to do it by hand?
     
  6. Offline

    ColonelHedgehog

    That's where the "crazy, obsessive dedication" comes in. ;P
     
    korikisulda and FerusGrim like this.
  7. Offline

    MrAwellstein


    o.o
     
    FerusGrim likes this.
  8. Offline

    Cirno

    I'm guessing he means that you should look at how the method is executed and follow the steps through your head rather than just reading the method name and quickly scanning over it.

    I'm intending for the below not to be rude, more of just criticism:
    Why didn't the plugin reviewers rename the gnp file to a png to see what the contents is?
    For starters:
    • If the reviewers noticed it, the plugin uses the map API to draw images.
    • The plugin comes with an image.
    • Image could contain pornography or some shock image or something that is totally against Curse ToS or worse, against the law.
    • If you renamed the image, it's unreadable/cannot be opened in Windows Picture Viewer, Paint, or Paint.Net.
    • The payload image is 151kb; kinda hefty for the resultant image that's 2kb and is 128x128 (negligible; they probably don't run a server and load the plugin into the server; maybe a select few original DBO staff might have in the past, but doing this is quite slow and cumbersome trying to find all the dependencies).
    • Again, if the reviewers noticed it, why does the plugin take a "GNP" file and then "convert" it to "PNG"? Why would a plugin contain a "custom format image" to begin with? The conversion code is a bit confusing (probably because I'm not exactly the brightest with byte manipulation), but it looks like all he's doing is just writing PNG's header, buffering some data, writing that data out (redundant?), and then reading until the next byte is equal to the end of the PNG file. For that matter, why is he even looking for the end of the PNG file? Isn't the entire conversion process just a basic renaming?
    Feel free to correct me if I'm wrong in any of the points given; I'm a bit sleepy right now and probably wasn't thinking straight.
     
    MrAwellstein and korikisulda like this.
  9. Offline

    FerusGrim

    The image is an actual image. You can view it on the Github page. It's in PNG format - the plugin converts it to GNP, before reading it. I'm not sure if this is actually necessary, or just something the author added as an extra, purposeful suspicion.
    If you renamed it to GNP, it wouldn't be readable as an image. However, in its native PNG format it's a normal image.
    The PNG format is a lossless image format. Meaning it's created to be as perfect after encoding as in design. So larger file sizes are normal.
    Same for me, it's 2am. :p
     
    korikisulda likes this.
  10. Offline

    RawCode

    There is absolutely no way to make bukkitdev or any other community driven file hosting safe.
    Before stuff resigned - multiple times botnets and other nasty stuff was approved, many plugins still have backdors that OP plugin creator, actually checks and other stuff makes bukkitdev LESS safe.

    When everyone are aware - no checks done, server owners will download only trusted plugins and only plugins verified to be safe by trusted users, when there is illusion of safety - server owners will download random crap and blame admins later.

    Any bukkit plugin ran under JVM ROOT and have full power over codeflow, its perfectly possible to *hide* harmful payload inside bytecode and execute it later, or go native level directly.
    Ever plugin with open source can have harmful payload inside.
     
  11. Offline

    korikisulda

    In some ways, in my opinion, it's more likely. It may deflect suspicion, and the binary doesn't necessarily have to be based on the released source code. It is, of course, also entirely possible that malicious code (or a precursor) could be disguised in the source code itself.

    While there are (and rightly so) concerns about the way Rocoo went about this, and I don't think vigilante auditing (Is that even a thing? Is now) is the way to do things, it would be useful to have a formal means to carry it out.

    My idea (such as it is) is for there to be a way to upload plugins containing malicious code that will enter the approval queue, but won't ever be accessible by server owners, even if they are approved. Again, just my idea :s

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: Jun 14, 2016
  12. Offline

    RawCode

    korikisulda
    Extremely easy to notice such kind of "ban" by logging out and checking how page looks like for everyone else.
     
  13. Offline

    korikisulda

    The suggestion is not intended as a 'ban' so much as a means to test malicious code detection without exposing any users to risk.
     
  14. Offline

    Anrza

    Ok, let's pretend we all stop blaming people and/or people admit that they made a mistake.
    What I think a lot of us wonders is:
    Did you learn from the mistake?
    Will it happen again?
    Are you working on getting more staff, or educating the current one?
    Is it safe to download plugins?
     
  15. Offline

    Googlelover1234

    To answer "is it safe": We can't really be sure. Any plugins I've uploaded after the BukkitDev team left are accepted next to instantly. I do provide safe plugins, however, I really don't believe it's getting checked. I think (if they actually are checking) they are only checking for simple setOp methods.

    To respond to
    most people don't think enough is being done. There are still thousands of servers running CraftBukkit, and those owners want to be somewhat protected against things like this.

    Overall, I don't believe the Curse team are looking over the projects. If, however they are, I think they skim through the code, looking for obvious forceops that 12 year olds code. If you are a server owner, I suggest not downloading any plugins that just seem sketchy. That brings me to another point, the dev.bukkit.org requires close to nothing for the description. I see people with 2 lines saying "This plugin does things. And stuff." and it's accepted?! That would be a great example on what not to install on your server.
     
  16. Offline

    Cirno

    It's necessary if you want to extract additional data from the GNP image; which is quite suspicious, to be honest. The author probably did add it to add suspicion, however, don't quote me on this as I have never talked to the author.

    It's not; download the compiled edition from GitHub's releases (alternativly, just add "/releases" at the end of the repos) and then try to change the file extension from gnp to png.

    Yet the resultent image is only 128x128 and 2kb? I don't get how 151kb would go to 2kb.
    This image: http://i.snag.gy/1gc1i.jpg (Note, I know this is JPEG; snag.gy does that)
    is only 43kb (http://i.snag.gy/TPzOQ.jpg)

    extra:
    Attempting to fix the image by purely editing the header results in (I can't upload the file anywhere):
    http://i.snag.gy/NjvoF.jpg
     
  17. Offline

    RawCode

    Its possible to store arbitrary payload inside arbitrary file, it's ever possible to assemble zip\jar archive that will not display it's real content for common zip application, ever high level security checks may be evoided by implementing special kind of time bomb that will activate harmful payload later or under specific conditions.

    I dont know how bukkitdev works, but, process can be automated in no time - automated check that perform search for blacklisted native calls, this can include disk\netIO, obviously include reflection\runtime\system invocations.
    Check extremely easy - constantpool of class + import section check, this can be done by sun.EMIT already in JDK, there is absolutely no way to evoid detection when such check is performed.
    If any blacklisted native invocations detected - plugin automatically rejected and developer asked to explain, why he used each native invocation.
    This also will help to fight plugins that evoid version barrier by monitoring reflection use.

    With reflection\runtime invocations banned, it trivial to detect "setop" or similar blacklisted bukkitAPI calls, there is absolutely no way to hide such invocation when reflection\runtime is disabled.

    Such actions will make bukkitdev safe by removing human factor, as long as plugin author not perfectly trusted, no plugin with potentially harmful code is approved.

    As long as plugins checked by hand - they not safe.
     
  18. Offline

    Kaelten

    We've learned a lot from this ordeal.
    Another incident is always possible, but I think that if this file was to be submitted today it'd not have made it through. Of course it's impossible to prove that as it's not repeatable.
    Our staff is always learning, adapting, and improving our processes.
    It is as safe as it's always been.
    We do inspect source for exploits. We don't moderate for quality, it's my opinion that that is not our place. We require a basic description, but not a novel. Ultimately popularity of a project filters out the vast majority of the rest.
    The old team concluded that automated means where inherently flawed. The human element can indeed be just as flawed as tools.
     
  19. Offline

    7NE

    What's sad is not that this malicious code got approved, but that people clamor for less restrictions (updaters, stats, external connections, etc), but get upset when something like this gets through. The previous staff had let InfiniteDispensers and the Nano plugin through, how would even less staff have an easier job?
     
  20. Offline

    MikeSheen


    Should we be retrospectively looking at already vetted plugins for similar exploits?
     
  21. Offline

    Techcable

    This plugin was very suspicious (and confusing) with its image manipulation. If you were bukkit-dev staff and saw a plugin with native code that would immediately rase suspicion too. If a bug happens in the decompiler which hides bytecode then a security breach is acceptable. I would also agree that the security checks of bukkit dev can cause some carelessness, but on the whole if you are a knowledgeable, security conscious admin then the approval process makes servers far more safe.

    If you reed the source code for those plugins, it is quite hard to find the issue, while the image manipulation going on in the Magix plugin is very suspicious considering what the plugin has to do.

    I hope that the current bukkit dev team learns that it is extremely important to review every single line of code of every single update to ensure safety.
     
    7NE likes this.
  22. This is what the former team does, and Curse says it does.
     
  23. Offline

    Techcable

    They claim they do but the approval times would suggest otherwise. I doubt a small team of people from Curse can approve plugins faster than the old team with more people that are more experienced.
     
  24. I wouldn't bet on that, there are a couple of factors making it realistic to be "faster":
    - Less plugins to approve: Long time since 1.7.10 was released + DMCA business. Most plugins reached a stablish state in comparison.
    - Full time work. No holidays.
    - Less experienced, thus going faster (would be a mistake, or just how things go then :p).
    - Biased approval (checking less strict for existing plugins, possibly judging "reputation" or so ~ potentially dangerous too, but much less than approving random projects + uploads).
     
  25. Offline

    MikeSheen


    Anyone?
     
  26. According to the latest posts by Curse staff, there have always been slip-throughs, even with people having downloaded content. If you assume that (say, during the the last year) some plugin(s) might have made it through unnoticed, probably with a more obfuscated functionality and less obvious to the server admin, then you always have to to stay minimally cautious.

    Manual review isn't perfect, no matter if former or new staff do it, just new staff decided to announce all plugins that make it through and get downloaded.

    Now what can the really cautious server owner do ... i don't see a simple approach.
    Lengthy... (open)

    The suggestion to compile the source code yourself probably saves you from poisoned uploads, but in theory the source code could be posioned (!) while the upload is not. I doubt that Curse staff checks the source repositories from the plugins, in addition decompiling the jar and checking that. So compiling the code yourself can only be a method for plugins that either are very active and popular and thus have a lot of review of the source, or for the case that you yourself review the source code and compile that then.
    For a conclusion, you probably have to find some acceptable way of judging updates (if you use jars from dev.bukkit.org or compile it yourself...), e.g. with sets of criteria:
    Basic:
    • Jar reviewed by Curse staff.
    • Open source.
    • Ticket managment is activated.
    • Appearance of the project makes sense. Prefer a somewhat mature or appropriate description and commenting. Careful if they are boasting about having solved stuff that other established plugins have not implemened or postponed because it's probably too complicated or even impossible. Careful if maintainers are pretending to be heroes too much.
    Basic, disadvantage for new developers and new plugins, or just small ones:
    • Open source repositories are cloned and watched by others, the more the better. Also if others are having more active projects on the same platform (e.g. GiutHub).
    • Project activity: Responses to tickets and requests, uploads for bug fixes and/or new MC versions.
    • Download count and duration of time passed, since the download has been approved. Don't be the first to download an exploit.
    • Knowing more about the developers. (Only trust real friends :p.)
    • Don't fall for "jokes" concerning how to run the plugin, e.g. "this plugin needs dependency jar XYZ, donwload at http://...".
    Advanced:
    • Code inspection: Have a look at their code. Once you have an overview, or the plugin has been around some while, checking commits can be more "efficient", though things can still be hidden. At least probing gets you an idea of what people are doing.
    • A full code + file review is of course "best".
    • Code Review: Do not forget to check maven dependencies and such, because a plugin could load other malicious content during compilation... you could probably as well decompile the jar file from dev.bukkit.org right away, doing about the same that Curse staff do.
    • Try to reproduce the binaries (insist on knowing what commit was built against, what compiler, OS, JRE/JDK, IDE versions). I am not sure if that is 100% possible, for jars you might have to compare on a file-by-file basis for contents, comparing some "manually" which have the version put in, e.g. plugin.yml. Also maven dependencies with "LATEST" version could be changed during compilation, though you might skip dependencies coming from trusted sources. All in all this is probably not a fun job.
    (These sets are probably incomplete.)

    For me all this hasn't changed with Curse staff taking over (i am not using all the mentioned criteria).
     
    nlthijs48 likes this.
  27. Offline

    xize

    I also got kinda confused about the policy with links like zippyshare or other sites like that on dbo plugin pages, I thought this was something what shouldn't be allowed than only Jenkins with a special disclaimer stating it is unoficial?, ive seen a few pages already containing this kind of links.

    also I wonder are stubs being checked aswell? maybe I say the wrong wording but let me refrain it a bit.

    when you open a .class file with notepad are the begin and the end being checked? that also contains when a plugin for example downloads something like a image binded with a executable on the end line of the file? you know you could make semi executables right? even when its a image its just looping to the questionable raw lines and execute it.
     
  28. As far as I'm aware, this is still against the rules. If you see pages like that, report them and see what Curse say.
     
Thread Status:
Not open for further replies.

Share This Page