And that is the point of this discussion, to determine how to do just that.
But considering it's such a difficult problem, we haven't figured out how to yet. Peter. ----- Original message ----- > > Message: 2 > Date: Wed, 21 Jan 2015 15:43:13 -0600 > From: "David M. Lloyd" <david.ll...@redhat.com> > > At some point, the responsibility *must* fall on the author of the > serializable class in question to avoid constructs which are exploitable > - like finalizers, and classes that can have side-effects before > validation can be completed. > > On 01/21/2015 02:26 PM, Peter Firmstone wrote: > > Don't forget that "null" may also be an invalid state. > > > > What I have learnt so far: > > > > 1. An attacker can craft a stream to obtain a reference to any object > > created during deserialization, using finalizers or circular links > > (there may be yet other undiscovered methods). > > 2. An attacker can craft a stream that deliberately doesn't satisfy > > invariants, in order to use an object to perform a function that > > wasn't intended by its developer. > > 3. Objects that interact with the stream directly using readObject et > > al, are often prone to DOS. Example, many objects read a length > > integer from the stream when creating an array or collection, > > without first validating it. > > 4. Objects that interact directly with the stream become an implicit > > part of the stream protocol. > > 5. Once you allow an object to be created, it's too late to > > invalidate invariants, unless the class is final and invariants > > are checked in every method call. > > 6. We need to be able to restrict classes used for deserialization to > > those we trust to check invariants properly (but we haven't > > provided a way for them to avoid object construction yet). > > 7. A static validator method can ONLY be used to check field > > invariants, not other objects and primitives that are read > > directly from the stream by an arbitrary Object during the process > > of deserialization. > > 8. The jvm can be modified to delay finalizer registration for > > deserialization. > > 9. Circular links can be disallowed. > > > > Ultimately however, all proposed changes add complexity, but when an > > object has been created with invalid invariants, an attacker will find > > a way. > > > > Thank you all for your time, this has been a very good discussion. > > > > Regards, > > > > Peter. > > > > On 22/01/2015 2:27 AM, Chris Hegarty wrote: > > > On 20/01/15 20:22, Peter Levart wrote: > > > > Hi Chris and Peter, > > > > > > > > It just occurred to me that the following scheme would provide > > > > failure atomicity for the whole Object regardless of whether > > > > readObject methods are used or not for various classes in > > > > hierarchy: > > > > > > > > > > > > - allocate uninitialized instance > > > > - call default accessible constructor of the most specific > > > > non-Serializable class > > > > - deserialize (by calling readObject methods where provided) the > > > > fields of all classes in hierarchy like normally > > > > (up to this point, nothing is changed from what we have now) > > > > - if deserialization fails anywhere, undo everything by setting > > > > all the fields in the Serializable part of the hierarchy to > > > > default values (null for references, 0 for primitives), abandon > > > > the object and propagate failure. > > > > > > I think this is a good idea, and I can prototype something to this > > > affect. > > > > > > -Chris. > > > > > > > > > > While deserializing, the object is in inconsistent state. If > > > > deserialization fails, this state is rolled-back to uninitialized > > > > state. finalize() can still get to the instance, but it will be > > > > uninitialized. > > > > > > > > > > > > Peter > > > > > > > > On 01/14/2015 01:58 PM, Peter Firmstone wrote: > > > > > > > > > > Hi Chris, > > > > > > > > > > Sorry, no. > > > > > > > > > > Currently when an ObjectStreamClass is read in from the stream, > > > > > the framework searches for the first zero arg constructor of a > > > > > non serializable class and creates and instance of the class > > > > > read and resolved from the stream, however it does so using a > > > > > super class constructor. > > > > > > > > > > Then from the super class down, fields are read in and set in > > > > > order for each class in the object's inheritance hierarchy. > > > > > > > > > > The alternative I propose, doesn't create the instance, instead > > > > > it reads the fields from the stream, one by one and without > > > > > instantiating them, if they are newly read objects, stores them > > > > > temporarily into byte [] arrays in a Map with reference handle > > > > > keys, otherwise it just holds the reference handle. > > > > > > > > > > What it does next is wrap this information into a caller > > > > > sensitive api, GetFields or ReadSerial instance, that is passed > > > > > as a constructor parameter to the child class serial constructor. > > > > > > > > > > The child class checks invariants and reads each field it needs > > > > > using a static method prior to calling a superclass constructor, > > > > > each class in the inheritance hierarchy for the object then > > > > > checks its invariants until it gets to the first non > > > > > serializable superclass. > > > > > > > > > > The benefit of this order is that each class is present in the > > > > > thread security context, so protection domain security and > > > > > invariants are enforced before instantiating an object. > > > > > > > > > > Hope this helps illuminate it a little better, regards, > > > > > > > > > > Peter. > > > > > > > > > > ----- Original message ----- > > > > > > Peter F, > > > > > > > > > > > > I am still struggling with the basic concept of you proposal. > > > > > > Let me > > > > > see > > > > > > if I understand it correctly. Does the following describe a > > > > > > similar scenario as you envisage: > > > > > > > > > > > > 1) For each Serializable type, T, in the deserialized types > > > > > > hierarchy, starting with the top most ( closest to > > > > > j.l.Object ), > > > > > > > > > > > > 1a) Read T's fields from the stream, fields > > > > > > > > > > > > 1b) validate(t, fields) // t will be null first time > > > > > > > > > > > > 1c) allocate a new instance of T, and assign to t > > > > > > > > > > > > 1d) set fields in t > > > > > > > > > > > > 2) Return t; > > > > > > > > > > > > So for each level in the hierarchy, an instance of a type is > > > > > > created only after its invariants have been checked. This > > > > > > instance is then passed to the next level so it can > > > > > > participate in that levels > > > > > invariants > > > > > > validation. > > > > > > > > > > > > If this scenario is along the same lines as yours, then I just > > > > > > don't > > > > > see > > > > > > how 1c above will always be possible. > > > > > > > > > > > > If we could somehow make the object caller sensitive until > > > > > > after deserialization completes, then could avoid having to > > > > > > try to allocate multiple instance down the hierarchy. > > > > > > > > > > > > -Chris. > > > > > > > > > > > > On 13/01/15 10:24, Peter Firmstone wrote: > > > > > > > Could we use a static validator method and generate bytecode > > > > > > > for constructors dynamically? > > > > > > > > > > > > > > The developer can optionally implement the constructors. > > > > > > > > > > > > > > static GetField invariantCheck(GetField f); > > > > > > > > > > > > > > Create a caller sensitive GetField implementation and add a > > > > > > > two new methods to GetField: > > > > > > > > > > > > > > abstract Object createSuper(); // to access superclass > > > > > > > objectmethods for inavariant checking. > > > > > > > > > > > > > > abstract Class getType(String name); > > > > > > > > > > > > > > Set fields from within constructors. > > > > > > > > > > > > > > The generated constructors are straight forward: > > > > > > > > > > > > > > 1. Call static method. > > > > > > > 2. Call super class constructor with result from static > > > > > > > method. 3. Set final fields > > > > > > > 4. How to set transient fields, implement a private method > > > > > > > called > > > > > from > > > > > > > within the constructor? > > > > > > > > > > > > > > Require a permission to extend GetField? > > > > > > > > > > > > > > > > > > > > -- > - DML > > > --------------
