feat(scanallclasses): create a scan all classes boolean#15
feat(scanallclasses): create a scan all classes boolean#15ThatGamerBlue wants to merge 1 commit into
Conversation
Not perf tested, shouldn't be too slow though. By default only allows classes globally allowed in the config. Not functional under modlauncher, couldn't see a way to get the transformer to indiscriminately apply to every class loaded. Wasn't sure if I should've added filtering to not process java.* classes, but theoretically these are exploitable so I decided to leave them filtered, might be a bad choice.
8d5d567 to
7000228
Compare
|
Not sure if this might be worthy of a rewrite w.r.t #10 to allow multiple PatchModules to target the same class, so we could have multiple patchmodules patching the SerializationUtils, grouped in the file by what mod adds the requirement for organization reasons eg: |
|
I would like to keep the config stable enough so that we don't break any older version of the patcher or that we need to maintain new versions of the config in this repo. But I think one approach to that problem could be the merging of PatchModules once one is requested for a specific class. So basically searching all PatchModules that have the class to patch in |
|
I think no matter what we do we're gonna be struggling with backwards compatibility, unless we just have one big entry in the config that contains every mod that uses the SerializationUtils, given older versions will just find the first one in the list and only allow those classes through, ignoring the rest. |
|
Hmm yeah true, totally forgot about that part :/ |
|
I think for now this PR should be fine as a stop-gap until we find most of the mods with issues, I don't think there's gonna be a way to have both the nicely formatted config file and maintain full backwards-compatibility, and the backwards-compat is definitely the most important of the two. |
few weeks ago, a critical vulnerability allowing arbitrary remote code execution on clients and servers (and therefore all connected clients on a server) was discovered in many Minecraft mods. We initially attempted to thoroughly and responsibly investigate the issue in order to publish a write-up and completely fix the vulnerability, but since a group named MMPA just published a blog post about the issue while omitting many crucial details about the vulnerability, we were forced to release a statement and attempt to fix the issue right away since they were currently putting millions of modded Minecraft users at risk. Information on the Vulnerability The vulnerability is caused by unsafe usage of the Java serialization feature in network packets sent by servers to clients or clients to servers, which allows instantiating any Java class that is loaded in the Minecraft instance. There was already a similar vulnerability in the past called "Mad Gadget". You can read more about that here: https://opensource.googleblog.com/2017/03/operation-rosehub.html https://foxglovesecurity.com/2015/11/06/what-do-weblogic-websphere-jboss-jenkins-opennms-and-your-application-have-in-common-this-vulnerability/ While there are just a relatively small number of attacks targeting this vulnerability in the wild, because of the significance of the vulnerability, it is extremely dangerous to play with the unpatched mods currently. Attackers have already attempted (and succeeded in some cases) to gain access to Microsoft tokens and browser session data. Since they can execute any code they want on a target system, the possibilities are endless. How Can I Protect Myself Against this Vulnerability? We developed a patcher that attempts to fix all mods that we know of (the list of mods is below). Should any more affected mods be discovered, a patch is as simple as updating the related config file. (We will publish a release that automates this for you) Version 1.3 of the patch now automatically uses the latest version of the config file and otherwise falls back to the local config file. If there is no config present, there should be an error informing the user that there are currently no patches applied. Minecraft Forge 1.7.x - Latest Download the JAR file from the latest release on the releases page - The fix is now also available on CurseForge and Modrinth Add the JAR file to your mods folder Download the latest config file from this Github repository and add it directly to your instances config directory Version 1.3 of the patch now automatically uses the latest version of the config file Any Other Instances Download the JAR file from the latest release on the releases page (or from CurseForge or Modrinth) and save it somewhere Add the following JVM argument to your client/server (refer to the documentation of the client/server launcher you are using for instructions on how to do this): -javaagent:<PATH TO SAVED JAR FILE> Download the latest config file from this Github repository and add it directly to your instances config directory Version 1.3 of the patch now automatically uses the latest version of the config file Affected Mods Unlike the blog post stated above, there are plenty of mods that are and could be affected by this issue. Although some of them are already fixed in their latest versions, these mods were exploitable in at least one older version. It is to be expected that several modpacks over the years just are not maintained anymore, but are still popular and loved within the community. Keeping this in mind, we are trying to help those people who still love running those modpacks and strive to keep them safe as they play. KEEP IN MIND THAT THIS LIST IS NOT COMPLETE! THESE ARE JUST THE MODS WE ARE CURRENTLY AWARE OF! CurseForge is already investigating the issue internally, so we hope we can get a nearly complete list of vulnerable mods and versions in the future. Also please keep in mind that this is a VULNERABILITY (one that has also affected many other software projects in the past), not something added to the affected mods with any malicious intent. So please do not harass any mod authors because their mods are affected, and also do not call affected mods viruses or something else. That is just completely wrong. We have moved our affected mods list to another location! See the link below: Affected Mods List This list will change frequently as we find more mods that could have vulnerabilities and as developers add patches to their own mods. If you want to help us keep this list up-to-date, please feel free to contribute! Technical Approach Our current approach to fixing the vulnerability is to have a config file with all currently known mod classes that need to be patched. SIB then checks these classes at runtime and replaces the exploitable calls to ObjectInputStream with our safe ClassFilteringObjectInputStream that only allows the deserialization of classes that are on an allow-list in the config file. This approach has the advantage that we only modify or block confirmed vulnerable uses of ObjectInputStream, while leaving other secure and miscellaneous uses of this class completely unaffected. As a result, the risk of potential game-breaking issues caused by over-blocking is heavily minimized. With this approach, we also do not have any incompatibilities with older Java versions. This means that we need to add all vulnerable mods to our config file so they get patched. Possible new cases of mods that are vulnerable are not patched unless they are added to the above-mentioned config file. We are currently working on a good approach to also patch all other uses of ObjectInputStream safely without breaking any mods in the process (see dogboy21#15 and dogboy21#18). Uses: ugba908@gmail.com Credits I am not the only one who was working on the investigation of the whole situation. Credits to anyone that was involved in this: Aidoneus (MineYourMind Server Network) Bennyboy1695 (Shadow Node Server Network) bziemons (Logistics Pipes Mod Developer) DarkStar (CraftDownUnder Server Network) Dogboy21 (MyFTB Server Network) Einhornyordle (MyFTB Server Network) emily (CraftDownUnder Server Network) Exa (Nomifactory Modpack Developer) HanoverFist (MineYourMind Server Network) Heather (MMCC Server Network) HellFirePvP (Astral Sorcery Mod Developer) Jacob (DirtCraft Server Network) Juakco_ (CraftDownUnder Server Network) Lìam (MineYourMind Server Network) MojangPlsFix (MyFTB Server Network) Niels Pilgaard (Enigmatica Modpack Developer) oliviajumba (CraftDownUnder Server Network) oly2o6 (All the Mods Modpack Developer / Akliz Server Hosting) PurpleIsEverything (Shadow Node Server Network) Pyker (Technic Launcher Developer) RyanTheAllmighty (ATLauncher Developer) Saereth (Modpack Developer) Sauramel (CraftDownUnder Server Network) ThePixelbrain (MMCC Server Network) Tridos (DirtCraft Server Network) Summary Release Note Documentation @dianecloud README 01a1ebc @coderabbitai coderabbitai Bot commented on Jun 7 • Review Change Stack No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info 📝 Walkthrough 🚥 Pre-merge checks | ✅ 4 | ❌ 1 ✨ Finishing Touches Comment @coderabbitai help to get the list of available commands and usage tips. @dianecloud dianecloud added this to @dianecloud's cripy-doodle on Jun 7 @github-project-automation github-project-automation Bot moved this to Todo in @dianecloud's cripy-doodle on Jun 7 coderabbitai[bot] coderabbitai Bot approved these changes on Jun 7 @github-project-automation github-project-automation Bot moved this from Todo to In Progress in @dianecloud's cripy-doodle on Jun 7 @dianecloud dianecloud added bug documentation enhancement good first issue question labels on Jun 7 @dianecloud dianecloud self-assigned this on Jun 7 @coderabbitai coderabbitai Bot commented on Jun 7 Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. @coderabbitai coderabbitai Bot mentioned this pull request on Jun 7 CodeRabbit Generated Unit Tests: Add unit tests for PR changes #2 Open @coderabbitai coderabbitai Bot commented on Jun 7 ✅ Created PR with unit tests: #2 dianecloud dianecloud commented on Jun 7 dianecloud left a comment Owner Author Commented in CodeRabbit Change Stack Merge info Changes reviewed 1 approving review by reviewers with write access. Some checks were not successful 1 failing, 1 pending, 3 successful checks failing checks pre-commit.ci - pr pre-commit.ci - pr — error during ci config pending checks DCO DCO — DCO successful checks CodeRabbit CodeRabbit — Review completed Socket Security: Project Report Socket Security: Project ReportSuccessful in 7s — Project Report: Success Socket Security: Pull Request Alerts Socket Security: Pull Request AlertsSuccessful in 3s — Pull Request #1 Alerts: Skipped No conflicts with base branch Merging can be performed automatically. You can also merge this with the command line.
Not perf tested, but (empirically) doing something similar to this in another project isn't noticably slower.
By default only allows classes globally allowed in the config. Not functional under modlauncher, couldn't see a way to get the transformer to indiscriminately apply to every class loaded.
Wasn't sure if I should've added filtering to not process java.* classes, but theoretically these are exploitable so I decided to leave them filtered, might be a bad choice.