NativeJavaList iterator unavailable in SafeStandardObject after commit b5e7f859

[Rhython]

Describe the bug
After the changes introduced by commit b5e7f85, the iterator for NativeJavaList is not available in the SafeStandardObject environment.

To Reproduce
Here is a code example that demonstrates the issue:

try (Context cx = Context.enter()) {
    Scriptable scope = cx.initSafeStandardObjects();
    scope.put("value", scope, List.of("a", "b", "c"));
    Object result = cx.evaluateString(scope, "let res = ''; for (let x of value) { res += x; } res;", "test", 1, null);
    System.out.println(Context.toString(result));
}

This code produces the following confusing error:

org.mozilla.javascript.EcmaError: TypeError: next is not a function, it is object. (test.js#1)

Expected behavior
This code runs as expected in release 1.8.0.

Additional context
This appears to be a regression introduced by commit b5e7f85. Please investigate if iterators support for NativeJavaList should be restored in the SafeStandardObject environment.

[rbri]

@Rhython i think this works as it should. initSafeStandardObjects() does not install all the java bridging stuff - because that is unsafe.
If you like to have the java objects support you have to use initStandardObjects() instead.

[Rhython]

@rbri I don't think so. NativeJavaList was designed to allow Lists to be used as JsArrays, and everything else works perfectly, except for the for-of loop. It's counterintuitive that for in works correctly while for of doesn't. Currently we can do this:

try (Context cx = Context.enter()) {
    Scriptable scope = cx.initSafeStandardObjects();
    scope.put("value", scope, List.of("a", "b", "c"));
    Object result = cx.evaluateString(scope, "let res = ''; for (let x in value) { res += value[x]; } res;", "test", 1, null); 
    System.out.println(Context.toString(result));
}

and it works fine.
The for...of loop is supported by JavaIterableIterator. Only objects that implement the java.lang.Iterable interface can work in the normal loop manner. There is no reflection involved, and it is completely controllable. I don't think this introduces any security issues.

[rbri]

@Rhython thanks for the detailed description, will have a closer look....

[rbri]

From the description of initSafeStandardObjects()

Initialize the standard objects, leaving out those that offer access directly to Java
classes. This sets up "scope" to have access to all the standard JavaScript classes, but does
not create global objects for any top-level Java packages. In addition, the "Packages,"
"JavaAdapter," and "JavaImporter" classes, and the "getClass" function, are not initialized.

My interpretation is that in this mode the whole LiveConnect to any java object should NOT be available to for a closed environment that only supports js types. But this is only may assumption.

And in fact this is not true - we do not initialize some classes but we still support java types because the context always has a WrapFactory and we always use that like in org.mozilla.javascript.ScriptRuntime#toObject()

// Extension: Wrap as a LiveConnect object.
Object wrapped = cx.getWrapFactory().wrap(cx, scope, val, TypeInfo.NONE);
if (wrapped instanceof Scriptable) return (Scriptable) wrapped;
throw errorWithClassName("msg.invalid.type", val);

For me this looks like a mess...

@aardvark179, @gbrail, @andreabergia what do you think? Can we please try to find a common understanding/definition for the save context.

[Rhython]

In fact, I found this sandbox mode didn't work at all. Even in the initSafeStandardObjects environment, I could still easily execute arbitrary code, including Runtime.exec/System.exit, etc. Is this considered a security vulnerability? I can provide a proof-of-concept (PoC), but I'm unsure whether it should be included in a public issue.

[gbrail]

@rPraml has done a lot of work in this area and would likely have a good perspective.

[gbrail]

as has @ZZZank!

[ZZZank]

This does look like a mess from my perspective. In theory there're 3 different levels of Java usage:

1. Pure JS environment, with no Java interop at all
2. Supports Java interop, but JavaImporter and such are not provided, so that users are limited to what the dev provides, instead of importing any arbitrary classes at will
3. Java interop + Rhino utils for Java interop

.initStandardObjects() provides a level-3 environment. The Javadoc of .initSafeStandardObjects() said it should provide a safe sandbox environment, but didn't specify whether Java interop should be allowed, possibly because doing for (let e of javaIterable) {} doesn't seem to be something dangerous whatsoever. As a result, old implementations just decided to provided a level-2 environment, which was then followed by later codes. Largely clear so far.

What makes things messy is, in my opinion, when rPraml is designing the new LiveConnect module, the goal of .initSafeStandardObjects() is "downgraded", from providing a level-2 environment, to providing a level-1 environment. Strictly speaking, this is just "clarifying undefined behaviour", but personally I don't like this. A new config entry in future LiveConnectBridge that tells whether Java interop should be allowed in .initSafeStandardObjects() is a good idea, changing existed behaviour silently is not.

In short, I agree that this is a bug and should be fixed

[rbri]

@ZZZank sounds reasonable and at least 'backward' compatible. Will try to spend some time to make unit tests for the safe environment and we can see what should work and what not. And also we can clarify the jdoc a bit.

Will also have a look at the WrapFactory, maybe this is an option for users to shild more java related functionality.

[aardvark179]

So I think most developers expect a level 2 environment where values passed by APIs can be acted upon (rather than being purely opaque). I'm all for trying to make this more well defined. At the moment we have options baked into the init method called, and at least 2 features on the context. I've just checked our internal fork and apparently we have 8, which I doubt are ever used independently.

I think this is something where it is worth us coming up with a proper plan before we start fixing things piecemeal.

@andreabergia and @0xe this feels like an area where we should take a look at what we do differently and if we can reduce it, or turn it into a more coherent set of changes.

[gbrail]

I agree that developers expect a "level 2" environment, but it depends on the context. I think that what some projects really want is to be able to embed Java code in a larger system and let their users define the code to execute, and that means that being able to execute arbitrary Java code should not be possible, and we don't have a very good system for that today. I think it's also important that we do what @***@***.*** ***@***.***> is proposing and have an option to completely remove all Java interoperability so that folks have the option of offering a pure JavaScript environment that only offers exactly the functionality that the developer is trying to offer, and nothing extra via Java reflection. That means using existing embedding techniques like the LambdaConstructor family of functions or the old Java annotations, or even just writing custom subclasses of ScriptableObject.

…

On Sat, Dec 6, 2025 at 7:01 PM Duncan MacGregor ***@***.***> wrote: *aardvark179* left a comment (mozilla/rhino#2192) <#2192 (comment)> So I think most developers expect a level 2 environment where values passed by APIs can be acted upon (rather than being purely opaque). I'm all for trying to make this more well defined. At the moment we have options baked into the init method called, and at least 2 features on the context. I've just checked our internal fork and apparently we have 8, which I doubt are ever used independently. I think this is something where it is worth us coming up with a proper plan before we start fixing things piecemeal. @andreabergia <https://github.com/andreabergia> and @0xe <https://github.com/0xe> this feels like an area where we should take a look at what we do differently and if we can reduce it, or turn it into a more coherent set of changes. — Reply to this email directly, view it on GitHub <#2192 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAD7I24G6A74J23QJL6YB734AMRWZAVCNFSM6AAAAACN3337WKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTMMRQHE2TGMJZG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[rPraml]

Just a heads-up: You may have noticed that I haven’t been very active lately. That’s because we’re expecting a baby :)
So if I happen to miss a mention or two, feel free to tag me again.

I did a lot of research the last years, how to get rhino "secure". See #1045 and others.

Unfortunately, I keep coming to the same conclusion: as soon as LiveConnect is involved, there are always ways to break out of the sandbox. Even mechanisms like ClassShutter do not offer 100% protection. I would even suggest deprecating or completely removing these mechanisms in version 2.0, as they create a false sense of security.

I agree, that in "level 1" with "pure js", a developer should expect, that the sandbox cannot be esacped.
(There may be other ways to abuse a system, e.g. endless loops, memory consumption, complex regexps...)

Unfortunately, in the past, LiveConnect objects were accidently initialized within initSafeStandardObjects()
As soon as NativeJavaObject.initMembers() does a reflective scan and exposes fields and methods, security is gone IMHO
(that's why I'll try to separate this in a own module to avoid such mistakes in future)

In my mind a "level 2" with a bit LiveConnect e.g. allow only lists or maps without a TopLevelImporter will not exist, as it provides no security. If you have access to a list in your js env you can do list.class.classloader.loadClass('java.lang.System') ... etc.

Concrete I see two options:

• Use "level 1" and write your own wrappers that are NOT based on NativeJavaObject or convert the data to NativeArray/Object
• Use "level 3" and go "all in" - you have to ensure, that only trusted javascript is executed or run the java process itself in a sandbox (e.g. docker container)

@Rhython please report your finding here: https://github.com/mozilla/rhino/security/advisories/new

[lTusErw6QCwMr2c6]

I think we can refer to the practices of similar products like Graal.js to some extent. Graal.js also supports Java interoperability, but it has strict restrictions on the use of reflection to call Java code. Unless the context is initialized with allowAllAccess(true), only class members annotated with @HostAccess.Export can be accessed by JavaScript. See https://www.graalvm.org/latest/reference-manual/embed-languages/#host-access

[aardvark179]

Yes, Graal & Truffle do take a very different approach to interoperability, and if we were starting from scratch it's probably the sort of model I would push for (it makes optimising things much easier), but it is very different to Rhino's current API. Given many of the major users of Rhino have a lot of code which interoperates with it at a fairly deep level, and the difficulty of changing that code is part of the reason they continue to use Rhino, we can't just change to a Graal style model.

[lTusErw6QCwMr2c6]

In fact, our current implementation is no different from allowAllAccess(true). You can regard this as the default mode, which ensures that existing users’ code is not affected. However, I believe we at least need an allowAllAccess(false) capability, which would give users who require it an additional option.