load() allows executing remote JavaScript code

[asiekierka]

A discussion over in Fabric's farlands led to the realization that Nashorn's load() can be used to execute JavaScript code, which injects arbitrary Java code, from an URL - a malicious entity could have some fun with it, probably. (Of course, this is already doable with regular old mods -- but the coremod API wants to be especially sandboxed.)

One way to solve this would be to just remove load():

jsContext = jsEngine.getContext()
jsContext.removeAttribute("load", jsContext.getAttributesScope("load"));

(While at it, quit() should be removed or replaced as well - right now it just quits the runtime)

[UpcraftLP]

(While at it, quit() should be removed or replaced as well - right now it just quits the runtime)

I'd rather have forge catch that and log an error / show an error message onscreen instead of just removing quit.

[modmuss50]

An easy way to test this is with the following in a coremod.js file

load("https://gist.githubusercontent.com/modmuss50/66db108d07d53fd371b3b3f2326d3d76/raw/396510d7b290efea9e5643917e01fa4f848a97ab/example.js")

check the log for the following:

[19:26:32.198] [main/DEBUG] [CoreMod/]: Loading CoreMod from C:\Users\mark\Documents\Games\Minecraft\1.13.2\TechReborn\build\resources\main\coremod2.js
hello internet
[19:26:32.593] [main/ERROR] [CoreMod/]: Error occurred initializing CoreMod
java.lang.NoSuchMethodException: No such function initializeCoreMod
    at jdk.nashorn.api.scripting.ScriptObjectMirror.callMember(ScriptObjectMirror.java:204) ~[nashorn.jar:?]
    at jdk.nashorn.api.scripting.NashornScriptEngine.invokeImpl(NashornScriptEngine.java:386) ~[nashorn.jar:?]
    at jdk.nashorn.api.scripting.NashornScriptEngine.invokeFunction(NashornScriptEngine.java:190) ~[nashorn.jar:?]
    at net.minecraftforge.coremod.CoreMod.initialize(CoreMod.java:32) ~[coremods-0.4.0.jar:0.4.0+17+382fa40]
    at net.minecraftforge.coremod.CoreModEngine.initialize(CoreModEngine.java:59) ~[coremods-0.4.0.jar:0.4.0+17+382fa40]
    at java.util.ArrayList.forEach(ArrayList.java:1257) ~[?:1.8.0_181]
    at net.minecraftforge.coremod.CoreModEngine.initializeCoreMods(CoreModEngine.java:53) ~[coremods-0.4.0.jar:0.4.0+17+382fa40]
    at net.minecraftforge.coremod.CoreModProvider.getCoreModTransformers(CoreModProvider.java:17) ~[coremods-0.4.0.jar:0.4.0+17+382fa40]
    at net.minecraftforge.fml.loading.FMLServiceProvider.transformers(FMLServiceProvider.java:126) ~[forge-1.13.2-25.0.70_mapped_snapshot_20190222-1.13.1-launcher.jar:25.0]
    at cpw.mods.modlauncher.TransformationServiceDecorator.gatherTransformers(TransformationServiceDecorator.java:49) ~[modlauncher-0.12.0.jar:?]
    at cpw.mods.modlauncher.TransformationServicesHandler.lambda$initialiseServiceTransformers$5(TransformationServicesHandler.java:62) ~[modlauncher-0.12.0.jar:?]
    at java.util.HashMap$Values.forEach(HashMap.java:981) [?:1.8.0_181]
    at cpw.mods.modlauncher.TransformationServicesHandler.initialiseServiceTransformers(TransformationServicesHandler.java:62) [modlauncher-0.12.0.jar:?]
    at cpw.mods.modlauncher.TransformationServicesHandler.initializeTransformationServices(TransformationServicesHandler.java:34) [modlauncher-0.12.0.jar:?]
    at cpw.mods.modlauncher.Launcher.run(Launcher.java:51) [modlauncher-0.12.0.jar:?]
    at cpw.mods.modlauncher.Launcher.main(Launcher.java:42) [modlauncher-0.12.0.jar:?]
    at net.minecraftforge.userdev.UserdevLauncher.main(UserdevLauncher.java:81) [forge-1.13.2-25.0.70_mapped_snapshot_20190222-1.13.1-recomp.jar:?]

You can see it fails to find the initializeCoreMod method, but above that it runs the js file from online. The game continues to start after this.

[asiekierka]

To clarify:

the issue with arbitrary load() is that you can swap the code from under the user
you could release a mod loading a remote .JS
which is fine for the first month
then once you have a decent amount of downloads swap it to something "fun" (or a hypothetical attacker could)

(Also, file:// works, which opens its own can of worms.)

EDIT: Again: this can all be done with a regular mod. But coremods are supposed to only allow ASM-related operations, so I consider this a bug at the very least.

[unascribed]

This is another deadly example of CVE-200103891 — "Code execution results in code execution"

This vulnerability must be immediately patched!

More seriously — mods aren't sandboxed at all, and the very existence of the coremod system/bytecode patching, unrestricted reflection, and Minecraft having no care taken toward it as far as security goes makes anything even adjacent to this a completely moot point. Mods can already download a URL and execute it, directly or indirectly. Coremods being able to do it should surprise no-one. (I've been out-of-the-loop for a bit but I doubt Forge has grown an entire sandbox in the ~month I've been gone.)

Better issue: "load() API left enabled, and its only use is making code less readable; should be removed". URL and URLConnection work. You can "exploit" this "vulnerability" with an if. File works as well.

[ChloeDawn]

I personally think the ability to load remote scripts is fine. It's only as problematic as general mods utilizing remote code/scripts/data at runtime to determine how they behave. And a benefit to network loading here would be the ability to push immediate hotfixes for bugs in your asm to clients and servers, which I feel could be quite useful.

[modmuss50]

Its not the end of the world, mods have been able to do this since the start of time. However the js coremod system was supposed to be more "locked down". This makes it a lot easier for people to make online coremods, in the past it would require a fair amount of work.

Due to how easy it is to prevent this I think it should be removed, but its nothing to worry about. At the end of the day you should trust what code you are downloading from the internet.

[asiekierka]

I think the issue still is allowing to execute remote JavaScript code; even if mods can do it normally, at least it takes a minimum of effort, and doesn't happen in a stage deliberately designed to be sandboxed off. Auto-updates should never happen to something as touchy as coremods.

Perhaps I have not communicated it correctly (I thought it's obvious you can still do this with regular old mods), but then if I hadn't tried people would have ragged on me for not reporting a potential security issue... whatever...

[asiekierka]

To re-iterate, because of all the FUD and people not understanding what words mean:

THIS DOESN'T LET YOU DO ANYTHING MORE THAN A NORMAL MOD WOULDN'T ANYWAY.

The principial reason I consider this a bug is because the coremod API promises to be sandboxed, and this - in my opinion, at least - breaks the conceptual limitations of said sandbox. Additionally, it's a bit more sneaky (especially with JS obfuscation) than downloading Java code off the Internet using "normal mod code".

[cpw]

Yup, load should be removed. I will fix that the next couple of days. But also, yes, mods have always been able to download stuff from the internet. Generally, those that do get discovered, and identified quickly. Disabling all connectivity from mods would be complicated and isn't something we're going to do.