Harden external worker IPC to loopback only

[Naveed8951]

Summary

A critical deserialization risk was identified in the external permutation worker IPC channel. The worker process deserializes objects from a socket. If that socket is reachable by untrusted parties, it could lead to arbitrary code execution.

Impact

If the port is reachable beyond localhost (e.g., in a misconfigured build host or a shared network), a malicious connection could send a gadget chain over Java serialization, resulting in remote code execution in the worker process.

[niloc132]

Thanks for the report - can you outline what the critical vulnerability is here? There are a few safeguards that appears to make this not actually a concern as far as I can see:

• port is randomized - attacker needs to predict the port that will be opened and the time it will be opened with a fairly small window window for success. I could be misreading the window here, but at best, this presents an opportunity for a denial of service because...
• too many connecting clients will crash the Compiler process - unless the attacker also has control over the localhost instance itself in some way to prevent them from launching?
• access is authenticated before arbitrary deserialization - each client must present its connection token "cookie", 16 bytes of random data converted into hex. Arguably this should be "SecureRandom" rather than "Random"

I don't disagree that as currently written, binding to localhost is likely acceptable/preferred - but how can the gadget chain be deserialized without knowledge of the preshared key/cookie and port?

Please make sure there is an issue to discuss first, so we don't waste your efforts in fixing something that might not be broken.

[Naveed8951]

The issue is not that an attacker can trivially guess the port and cookie. The issue is that the worker exposes Java ObjectInputStream behind a network listener at all. Once a deserialization sink is reachable via a socket, security guarantees depend entirely on environmental assumptions rather than a hard boundary.

Addressing the specific points:

Port randomization and a short listen window reduce likelihood but do not form a security boundary. If the socket binds to a non loopback interface, the attack surface exists for the duration of the listener. In CI runners, shared build hosts, containers, or misconfigured machines, this exposure is realistic.

The cookie check reduces risk but does not change the nature of the sink. The connection is accepted first, and the cookie is transmitted in cleartext and not bound to the peer or transport. If the cookie is leakedlogged, reused or bypassed the deserialization path becomes reachable.

The question is not whether an attacker can exploit this without any local foothold today. The question is whether unsafe deserialization is reachable outside a strictly local trust boundary. In security terms ObjectInputStream is treated as an RCE primitive and is only considered safe when confined to localhost.

Binding the listener to loopback removes the exposure entirely and enforces the intended trust model. That is why this change is about eliminating a risky class of behavior rather than responding to an actively exploitable bug.

[tbroyer]

Is this necessary given the .bind() of the socket?

[Naveed8951]

Yes, that is fair.

If the socket is always bound to loopback, then this check is redundant in the normal case. I added it as a defensive guard to fail closed if the bind behavior ever changes.

I am fine removing it and relying solely on the bind if that is preferred.

[niloc132]

I misread what accept() is doing - it isn't functioning like a listener here, just functionally validating that PermutationWorker.compile is called no more than the expected number of times on the right number of external worker instances.

---

The issue is that the worker exposes Java ObjectInputStream behind a network listener at all. Once a deserialization sink is reachable via a socket, security guarantees depend entirely on environmental assumptions rather than a hard boundary.

Are you certain about that? While the OIS is indeed instantiated, no objects are read from it (so no objects are even deserialized) until after the cookie bytes are read from the underlying IS (plus or minus the buffer):

gwt/dev/core/src/com/google/gwt/dev/ExternalPermutationWorkerFactory.java

Lines 125 to 135 in 05cee59

	// Verify we're talking to the right worker
	String c = in.readUTF();
	if (!cookies.contains(c)) {
	throw new TransientWorkerException("Received unknown cookie " + c,
	null);
	}
	out.writeObject(astFile);
	// Get the remote worker's estimate of memory use
	long memoryUse = in.readLong();

I don't even think we need an OIS at all except for the readObject to deserialize the exception data - but the read calls are readUTF() and readLong() (gated behind validity of the cookie anyway), and as far as I can tell neither pathway leads to any readClass/resolveClass, much less actual object deserialization?

If we were using readObject() and casting to String, that would clearly be vulnerable, but instead we're just using DataInput.readUTF(), which OIS's internals are buffering for us. Given that we're using DataInput methods, OIS can't deserialize objects preemptively - it would get confused due to the other bytes that could be present on the wire first, and wouldn't see the class descriptor that was expected.

Am I misreading OIS's javadoc or implementation here? I'll try to validate those assumptions shortly.

I'd like to ask again what the critical deserialization issue is - are you sure this is even possible given that the attacker always knows the port (but not the cookie) and never has to race against another connecting worker client? Is there an actual PoC, or is this just hypothetical?

---

My chief concern with this fix is the cases where "localhost" doesn't quite mean what you expect - or, that it isn't (in this day of docker/etc ports being bound as localhost) actually sufficient to protect us here.

Instead, with the temporarily accepted assumption that there actually is an RCE vulnerability:

• Improve cookie entropy with SecureRandom
• Don't even create a (StringInterning)OIS until after cookie has been checked, but read unbuffered from the input stream instead. That could be done in CountedServerSocket, and no need for hex encoding nor length prefix, since the secret length is fixed at 16 bytes, just need to read to a buffer and compare.
• Consider dropping and ignoring connections that don't present a valid cookie, to mitigate a potential DoS, and keep waiting for a valid cookie

With these in place, we shouldn't need to worry about which socket is being connected to at all?

[vjay82]

Don't exactly understand what this is but would it still be possible to open a dev/CodeServer backed website/app on another host for quick testing?

[niloc132]

Don't exactly understand what this is but would it still be possible to open a dev/CodeServer backed website/app on another host for quick testing?

Yes - this is just for the compiler to start another JVM for compilation work, rather than using threads. It won't run during dev mode at all (required to be a single permutation), but would only apply when running the compiler with localWorkers set to more than one, and the permutation worker factory set to use the class in question (which is the default).

[Naveed8951]

Yes. This does not affect dev mode or CodeServer usage.

This only applies to the compiler when running with multiple local workers. Dev mode uses a single permutation and does not use this worker path at all.

[niloc132]

actively exploitable bug

One more try: do we actually have any sample payload that can trigger deserialization pathways on the server without knowledge of the cookie (assuming knowledge of the port)?

I spent some time reading ObjectInputStream sources and debugging our usage of it (through readUTF()), and cannot find any pathway by which a serialization gadget could be actually parsed during the course of reading the cookie bytes.

I'm loathe to suggest it, but between the actual bug not existing, ignoring the format, non-responsive to requests about the actual issue, and the use of markdown... is this a real human submitting this?

It's probably not a bad change, but I think its neither necessary nor sufficient if the serialization bug is real through readUTF(), and if it does indeed do nothing, its not without risk.

I'd prefer to close this and harden the other avenues here - better randomness, allow server to continue with an invalid/malicious client, and to avoid any doubt about it, read cookie bytes with a separate reader than OIS. Unless @Naveed8951 can outline the actual way this can be exploited or even reach theoretically exploitable code (e.g. OIS.readObject and friends before cookie was validated).

#10256 for follow up should we close this.