What about potentially vulnerable classes that *aren't* in the config?
SuperStormer opened this issue ยท 6 comments
Forgive me if I misread the code, but
suggests that OIS is only redirected to the filtered version in classes that are covered by one of the patchModules in the config. Wouldn't this leave open the possibility that a vulnerable class was missed and remains unpatched?possibly solved by #15 ?
As already mentioned, our patch only applies to the known vulnerable classes in the config file so we're able to provide a valid deserialization allowlist for all cases instead of possibly breaking the mods behavior when we outright block all attempts of deserialization.
I will take a look at #15 (once I've got some spare time) which could be a valid option for those willing to take the risk of potentially breaking mods.
Yes. Only known exploits are fixed by the mod, there might still be a ton of affected mods out there we haven't discovered yet and all the exploits that come with them won't be fixed by our mod. As we stated multiple times, we got rushed to provide this fix early, we were just in the process of establishing contact to curseforge and modrinth so we would have two massive mod databases to scan for exploits but now we have to use what we got and this is it.
Still, the exploits that we could fix with this mod are way better than being completely unprotected.
Why not use the serialization filtering feature? That avoids the need to rewrite bytecode and it applies to the whole process:
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/io/ObjectInputFilter.html
See "Serialization Filters" here:
The problem with ObjectInputFilter is that it's only available in newer Java versions (>9 and >8u121 according to this blog article).
And while I totally know that 8u121 was released in January 2017, we are also aware that there are still Launchers shipping a version as old as 8u51, which would completely break a fix depending on that feature.
Then we also noticed that many mods use OIS internally to persist some state (so not in the direct network path) which we would also completely break with that approach probably. I know that our approach with just fixing the classes that we know of definitely isn't perfect, but I think it's a good compromise between security and literally breaking any mod in existance.
