On 9 November 2015 at 09:21, Thomas Neidhart <thomas.neidh...@gmail.com> wrote: > Hi all, > > please review the proposed fix for this issue here: > > http://svn.apache.org/viewvc?view=revision&revision=1713307
Those changes look workable to me. The main issue from my reading is that real users of serialisation with InvokerTransformer should be able to set it up, but by default it should not be accessible because it is an entry point into every part of the classpath, whether they are serializable or not. Anyone who does actually set it up possibly still doesn't understand the full security implications, but it is out of our hands by then. One guide that I read yesterday that highlighted the number of ways serialisation bugs can occur is: http://www.oracle.com/technetwork/java/seccodeguide-139067.html#8 In particular, one further step is that we may need to do an audit of all the serializable classes to see if we need to either change some variables to transient, remove "Serializable", or add custom serialisation/deserialisation as in this case. We may not be able to change fields or Serializable until a future revision to maintain compatibility, but it could help with the public image of commons being heavily reused and actively caring about security by responding promptly as we are doing now. One small comment, in the tests, you may want to use try-finally to set and clear the system properties, as if the test fails, the system property settings may leak out to other tests. It is a minor thing, because it only has any effect if the test fails, and only on one other test in this case. It is a bigger deal in tests where the test changes a widely used system property and must always change it back for other tests to succeed. Cheers, Peter --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
