vrjdev commented on PR #58016: URL: https://github.com/apache/spark/pull/58016#issuecomment-6299917013
> Thanks for the update, @vrjdev. I confirmed that all the earlier comments are addressed: `assertEquals` on the resolved class, the prefix-boundary spoof test, the collapsed and renamed reject test, the [SPARK-59544](https://issues.apache.org/jira/browse/SPARK-59544) follow-up reference, the launcher subpackage test, and the updated description. > > A few remaining non-blocking points, inline. One more note for [SPARK-59544](https://issues.apache.org/jira/browse/SPARK-59544) rather than this PR: `FilteredObjectInputStream` only overrides `resolveClass`, so for a serialized dynamic proxy the interface names go through the default `resolveProxyClass` and are never checked against the allow-list. In a quick local experiment, a proxy implementing `java.util.concurrent.Callable` with a serializable handler in an allowed package deserialized fine; only the `java.lang.reflect.Proxy` super descriptor and the handler class hit `resolveClass`. The handler class still has to pass the filter, so I don't see a practical exploit, but it may be worth mentioning in [SPARK-59544](https://issues.apache.org/jira/browse/SPARK-59544), since the exact-package fix would also reject `java.lang.reflect.Proxy`. Thanks for the thorough re-review, @sarutak. All three inline comments are addressed in the latest commit, and the PR description now lists `java.lang.Long` instead of `java.lang.String`. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
