236117 - Context sealing API for Rhino

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Igor Bukanov]

Since changing Context parameters can significantly alter script execution, I
suggest to add to Context a new API to seal Context instance so any future
attempt to change its parameters would throw an exception.

It would not only prevent bugs in applications but also allow to prevent
security breaches as currently Rhino has no protection against combined attack
of untrusted Java and JavaScript code. For example, low-privileges jar executed
as a part of untrusted script can call Context.exit/Context.enter to create
Context without security controller and use to produce interpreted scripts
having the same privileges as Rhino code.

Comment 1

[Igor Bukanov]

Targeting 1.5R5

Comment 2

[Igor Bukanov]

Attachment: Sealing implementation

The essential part of the patch is the following API:

    /**
     * Checks if this is a sealed Context. A sealed Context instance does not
     * allow to modify any of its properties and will throw an exception
     * on any such attempt.
     * @see #seal(Object sealKey)
     */
    public final boolean isSealed()
    {
	return sealed;
    }

    /**
     * Seal this Context object so any attempt to modify any of its properties
     * including calling {@link #enter()} and {@link #exit()} methods will
     * throw an exception.
     * <p>
     * If <tt>sealKey<tt> is not null, calling
     * {@link #unseal(Object sealKey)} with the same key unseals
     * the object. If <tt>sealKey<tt> is null, unsealing is no longer possible.

     *
     * @see #isSealed()
     * @see #unseal(Object)
     */
    public final void seal(Object sealKey)
    {
	if (sealed) onSealedMutation();
	sealed = true;
	this.sealKey = sealKey;
    }

    /**
     * Unseal previously sealed Context object.
     * The <tt>sealKey<tt> argument should not be null and should match
     * <tt>sealKey<tt> suplied with the last call to
     * {@link #seal(Object)} or an exception will be thrown.
     *
     * @see #isSealed()
     * @see #seal(Object sealKey)
     */
    public final void unseal(Object sealKey)
    {
	if (sealKey == null) throw new IllegalArgumentException();
	if (this.sealKey != sealKey) throw new IllegalArgumentException();
	if (!sealed) throw new IllegalStateException();
	sealed = false;
	this.sealKey = null;
    }

with intendent usage like:

private Object sealKey = new Object();

Context cx = new Context();
customize(cx);
cx = Context.enter(cx);
cx.seal(sealKey);
try {
...
} finally {
    cx.unseal(sealKey);
    Context.exit();
}

Now any attempt to call Context.enter/exit or modify Context parameters would
fail unless sealKey is known so the trusted code still can call:

cx.unseal(sealKey);
try {
    cx.setOptimizationLevel(9);
} finally {
    cx.seal(sealKey);
}

Another part of the patch is to make most of the read methods in Context final
as additional protection against "tailored" Context objects.

Comment 3

[Igor Bukanov]

Attachment: Patch without unrelated code

The patch from attachment 142684 [details] [diff] [review] is not compilable and included unrelated code
for bug 236193. This is a proper version.

I also forgot to document another patch feature: new method
Context.disableStaticContextListening() :


    /**
     * Disable notifications of listeners registered with
     * {@link #addContextListener(ContextListener)} about Context events.
     * All currently registered listeners will be removed and any subsequent
     * call to {@link #addContextListener(ContextListener)} will throw an
     * exception.
     * <p>
     * Embedding may use this method to prevent Context exposure to potentially

     * untrusted code.
     */
    public static void disableStaticContextListening()
    {
	synchronized (staticListenersLock) {
	    disabledContextListening = true;
	    staticListeners = null;
	}
    }

Although Context.addContextListener is a convinient method, as any method
operating on static data it can be exploited if an application allows to run
untrusted jars and using Rhino for internal purposes. In such cases the
application may be willing to drop convinience of Context.addContextListener
for a better porotection againt hosile code.

Comment 4

[Igor Bukanov]

I committed the last patch