DanielLeens commented on PR #11557: URL: https://github.com/apache/seatunnel/pull/11557#issuecomment-5363728926
Thanks @SEZ9. Head is still `e62beaaadf98132b7058f6f39de83238f4cfcb5e`, no new commit, so this stays a reply. On the deserialization finding: I went and read `ManagedSourceRegisterOperation.java` specifically to check this against source before answering, and I want to push back on the "unrestricted Java deserialization" framing, with evidence, rather than just accept or defer it. `ManagedSourceRegisterOperation` doesn't do raw Java serialization (`ObjectInputStream`/`readObject()` off an arbitrary class name — the pattern behind real deserialization-RCE gadget chains). It's: - `extends TaskOperation implements IdentifiedDataSerializable` (`TaskOperation.java:34`) — every field is written/read through Hazelcast's own typed `ObjectDataOutput`/`ObjectDataInput`, and every non-primitive field goes through `writeString`/`readString` or `writeObject`/`readObject` against Hazelcast's `IdentifiedDataSerializable` machinery, not JDK serialization. - The one object field, `readerLocation`, is a `TaskLocation`, which itself `implements IdentifiedDataSerializable, Serializable` (`TaskLocation.java:34`) — so it decodes through the same closed factory, not JDK object graph walking. - That factory is closed and explicit: `TaskDataSerializerHook.createFactory()` dispatches on `classId` through a fixed `switch`, with `case MANAGED_SOURCE_REGISTER_OPERATION: return new ManagedSourceRegisterOperation();` (`TaskDataSerializerHook.java:119`, `:203-204`). An attacker on the wire can only ever cause instantiation of one of the classes this switch enumerates — there's no class-name-driven, attacker-chosen instantiation, which is the actual property that makes JDK deserialization dangerous. - And this isn't new or specific to this PR: the identical `field = in.readObject();` pattern (no type-check overload) is the dominant convention across this exact package on unmodified `dev` — `SinkRegisterOperation.java:86-87` (`writerTaskID`/`committerTaskID`, the closest structural analog: another "*RegisterOperation" reader/writer handshake), plus `AssignSplitOperation`, `BarrierFlowOperation`, `CloseRequestOperation`, `DeployTaskOperation`, `SendConnectorJarToMemberNodeOperation`, and others. `ManagedSourceRegisterOperation` is following the codebase's existing wire-protocol convention exactly, not introducing a new decoding pattern. So I don't currently see grounds for a HIGH-severity security blocker here — the mechanism is the same closed, factory-scoped Hazelcast `IdentifiedDataSerializable` protocol the whole Zeta cluster-internal RPC layer already relies on for every operation, and this class isn't doing anything more permissive than its siblings. If you have a specific threat model in mind that survives the closed-factory constraint above (e.g. a way to make `classId` resolve to a different registered operation than intended and cause harm through that type confusion, rather than arbitrary code execution), I'd genuinely like to see it spelled out — that would be a narrower but still real robustness point (a missing `in.readObject(TaskLocation.class)` type-check on `readerLocation`, matching what `TaskOperation`'s own base-class field already does, would turn a wrong-classId case into a clean cast failure instead of a mismatched object silently flowing further) — but as stated, "unrestricted Java d eserialization" isn't what this code does, and I don't think it should stay on the blocker list under that framing. Everything else in your comment I agree with as-is: no complaints on the scope call, and yes — once a real commit lands (starting with the Issue 1 `PREPARE_CLOSE` hang, which is still the most severe open item on my own list), I'll note the exact head it applies to so the next round stays this cleanly scoped. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
