Skip to content

Conversation

@lonevox
Copy link

@lonevox lonevox commented Nov 1, 2025

When Java Class objects get passed from JS to Java they are incorrectly wrapped in NativeJavaObject instead of NativeJavaClass. This PR fixes that.

When Java Class objects get passed from JS to Java they are incorrectly wrapped in NativeJavaObject instead of NativeJavaClass. This PR fixes that.
@ZZZank
Copy link
Contributor

ZZZank commented Nov 4, 2025

Well I think this is intentional, wrapping any Class<?> object means that any method/field that returns a Class<?> will possibly return a usable reference to banned classes, causing security problem

@MaxNeedsSnacks
Copy link
Member

Well I think this is intentional, wrapping any Class<?> object means that any method/field that returns a Class<?> will possibly return a usable reference to banned classes, causing security problem

Yeah it's definitely not a solution to just wrap every class as NJC, it would likely have to go through the class filter somehow (but iirc that is implemented on KubeJS' side?)

@lonevox
Copy link
Author

lonevox commented Nov 5, 2025

The behavior on the JS side doesn't change at all. All loadable Java classes are already accessible by design, like this:

let $ListTag = Java.loadClass("net.minecraft.nbt.ListTag")

const listTagClass = $ListTag.__javaObject__
console.log(listTagClass)

You can see all the Class methods if you do:

console.log(JSON.stringify(listTagClass, null, 2))

You can call most Class methods on it, but importantly, you can't call any caller sensitive methods, which are the methods that do the actual potentially security-breaking reflection. So basically you can just call isArray() and other similar functions.

I actually found this NativeJavaClass bug while making a mod to allow for reflection in kube: https://github.com/lonevox/KubeJS-Reflected
Obviously I wouldn't PR any code that aids in reflection. This just fixes the issue of passing a Class to JS results in a NativeJavaObject, which can't be passed back to Java as a Class, so you get a confusingly widened type if your kube plugin is expecting a Class from JS (which mine does expect often).

Also you cannot ever get a usable reference to banned classes from Java. The class filter doesn't allow banned classes to be sent from Java to JS. It doesn't matter if it's wrapped as a NativeJavaObject or a NativeJavaClass. My mod has to circumvent the class filter to make reflection work.

@ZZZank
Copy link
Contributor

ZZZank commented Nov 7, 2025

This just fixes the issue of passing a Class to JS results in a NativeJavaObject, which can't be passed back to Java as a Class

I believe there's no such issue. NativeJavaObject and NativeJavaClass are all implementations of Wrapper, so even if its Class is not wrapped to NJC, it can still be unwrapped to the same object when passed back to Java. I guess your problem should be relying on NativeJavaClass for determining if an object is Class:

Class<?> clazz;
if (jsObj instanceof NativeJavaClass njc) {
    clazz = ...;
}

I would recommend:

Class<?> clazz;
if (jsObj instanceof Wrapper wrapper && wrapper.unwrap() instanceof Class c) {
    clazz = c;
}
// or alternatively:
if (Wrapper.unwrapped(jsObj) instanceof Class c) {
    clazz = c;
}

Also I'm curious, JS objects will be automatically unwrapped and transformed when accessing Java method/field, why would you need to unwrap it manually?

@lonevox
Copy link
Author

lonevox commented Nov 8, 2025

Also I'm curious, JS objects will be automatically unwrapped and transformed when accessing Java method/field, why would you need to unwrap it manually?

Because specifically Class<?> objects don't unwrap automatically unless they're from a NativeJavaClass, so having a function like the following in a kube plugin doesn't work:

private void expectsClass(Class<?> clazz) {
    System.out.println(clazz.isArray());
}

I'm unsure if the above is even possible without circumventing the class filter though, so I admit this is a very specific issue that probably only I will even run into. So if it's worth merging, idk, I currently fix it myself with a mixin so I'm not that bothered. Just thought I might as well contribute the fix.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants