Isn't the issue with non-serializable superclass constructor call this
one? :
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2008-5353
If so - I don't really see how it relates to River - to be able to
expoit this kind of vulnerability an attacker must have already
downloaded and run his code - otherwise the exploiting subclass could
not have been loaded hence no construction would take place.
I think if we can make sure only trusted (whatever that means :) ) code
is ever run - we don't have to do anything more with (de)serialization -
except making DoS more difficult by restricting size of the stream.
Thanks,
Michal
Peter Firmstone wrote:
> Continuing on ...
>
> Lets say for example, we have a secure OS and we provide a service on
> a public port and we have a determined attacker attempting to use
> deserialization to take over our system or bring it to its knees with
> denial of service.
>
> We know this is relatively easy with standard ObjectInputStream.
>
> When we invoke a remote method, the return value is what you call a
> root object, that is it's the root of a serialized object tree,
> originating through the root object, via its fields.
>
> Most Serializable objects have invariants.
>
> We can consider the java serialization stream format as a string of
> commands that allows creation of any object, bypassing all levels of
> visibility. That is an attacker can create package private classes or
> private internal classes, provided they implement Serializable. So
> think of Serializable as a public constructor that lacks a context
> representing the attacker. That's right, there is no context
> representing the remote endpoint in the thread call stack.
>
> Now we can place restrictions on the stream, such as a requirement
> that the stream is reset before a limit on the number of objects
> deserialized is reached. We can also place a limit on the count of
> all array elements read in from the stream, until the stream is
> reset. These measures ensure the stream cannot go on forever, they
> also prevent stack overflow and out of memory errors. At some point
> the caller will regain control of the stream without running out of
> resources.
>
> But getting back to the previous problem, the attacker has command
> line access to create any Serializable object.
>
> Now most objects have invariants that should be satisfied, for correct
> functional state, but a major problem with Serialization is it creates
> an instance, using the first zero arg constructor of the first non
> serializable superclass. Yep that's right, that could be ClassLoader,
> you clever attacker you! The poor old object hasn't even had a chance
> to check it's invariants and it's game over, that's not fair!
>
> This will never do, I had to create a new contract for atomic object
> construction:
>
> 1. The class must implement Serializable (can be inherited) AND one
> of the following conditions must be met:
> 2. The class must be annotated with @AtomicSerial OR
> 3. The class must be stateless and can be created by calling
> class.newInstance() OR
> 4. The class must have DeSerializationPermission
>
> Now if the class is annotated with @AtomicSerial, it must also have a
> constructor signature that has public or default visibility:
>
> SomeClass(GetArg arg) throws IOException{
> // The first thing I must do is to call a static method that
> checks invariants, before calling a superclass constructor, the static
> method should return the argument for another constructor.
> }
>
> Some simple rules for our object input stream:
>
> 1. Though shalt not publish a reference to a partially constructed
> object.
> 2. Though shalt not publish a reference if an object fails
> construction, where readObject, readObjectNoData and readResolve
> methods are considered to be constructors for the purpose of
> deserialization of conventional Serializable objects.
> 3. Though shalt not attempt to construct a Serializable object that
> doesn't have an @AtomicSerial annotation, or has serial fields
> (state) and doesn't have DeSerializationPermission.
> 4. Only call a protected constructor if the class doesn't implement
> @AtomicSerial and has DeSerializationPermission.
> 5. Do not honour the standard java Serialization construction
> contract (not all Serializable classes can be constructed even if
> they have DeSerializationPermission).
> 6. If a standard Serializable class has DeSerializationPermission for
> it to be constructed it must have a zero arg constructor or a
> constructor that accepts null object arguments and default
> primitive values.
> 7. If an any object in a serialized object graph fails it's invariant
> checks, deserialization of the object graph fails at that point
> and control is returned to the caller, by way of an
> InvalidObjectException (a child class of IOException).
> 8. Honour the standard java serialization stream protocol.
> 9. Count number of objects received and throw a
> StreamCorruptedException if limit is exceeded.
> 10. Count number of array elements received and throw
> StreamCorruptedException if limit is exceeded.
> 11. readResolve() can be called on @AtomicSerial instances but,
> readObject() is never called and neither is readObjectNoData().
>
> Obligations of our object output stream:
>
> 1. Reset the stream before the limit is reached.
> 2. Replace java collections and maps with safe @AtomicSerial
> implementations, it is the developers obligation to replace them
> with their preferred implementations during construction, these
> are functional but are only immutable containers for keys, values
> and comparators.
> 3. Honour java's serial stream protocol.
>
> Some simple rules for developers implementing @AtomicSerial:
>
> 1. Check all invariants from a static method called by your class
> constructor prior to calling a superclass constructor.
> 2. Check types of values in collections and maps and keys in maps.
> 3. Check field types.
> 4. Copy or clone arrays and collections.
> 5. Do not call a super class constructor until all invariants have
> been checked.
> 6. Handle failed invariants by throwing an InvalidObjectException,
> again make sure you did not call the super class constructor.
> 7. Implement the interface ReadObject if you want to duplicate
> existing readObject method functionality, this will allow you to
> communicate with the stream prior to calling a super class
> constructor, so you can check invariants. Annotate a static
> method that returns an instance of ReadObject with @ReadInput.
> 8. Your ReadObject instance can be retrieved by from GetArg, your
> ReadObject instance will have read the stream prior to your
> constructor having been called.
> 9. Don't call defaultReadObject() from your ReadObject implementation!
> 10. Check all fields can be retrieved, prior to calling a super class
> constructor.
> 11. Beware of the implicit super class constructor.
> 12. Enum, and Proxy instances are considered secure, don't bother
> subclassing proxy, your class won't be deserialized.
> 13. InvocationHandler's need to implement @AtomicSerial.
>
> Note an object instance is not created until the default constructor
> in java.lang.Object is called.
>
> Golden rules:
>
> 1. Do not create arrays or collections blindly by reading in an
> integer or long, length or size from the stream to pass as an
> argument to an array or collection constructor.
> 2. Let the stream be responsible for array creation.
> 3. Clone or copy arrays and collections.
> 4. Clone or copy mutable objects, these may be shared by other
> objects in the stream.
> 5. Don't grant DeSerializationPermission unless you're really really
> sure you know what your doing, you must have access to the source
> code of the object you want to deserialize, you must make sure it
> is secure, if in doubt ask first.
>
> Now before anyone goes off thinking "Oh no I don't need this
> functionality, do I have to use it?" The answer is no, it will be set
> via configuration constraints. If an endpoint doesn't support
> AtomicValidation, it will prevent ObjectInputStream creation. If an
> object in the stream doesn't support it, deserialization will fail and
> control will be returned to the caller.
>
> Other than configuration, this is invisible to clients, only service
> api parameters and return values and private service classes, such as
> smart proxy's and proxy verifiers need implement it.
>
> Note: I have tested both Reggie and Outrigger, standard serial form is
> preserved for all objects, all tests pass with AtomicValidation.YES
> enabled.
>
> Clients will be able to be configured with InvocationConstraints;
> AtomicValidation.YES, Integrity.YES and Confidentiality.YES.
>
> The client can also require DownloadPermission by specifying either of
> the properties:
>
> net.jini.loader.ClassLoading.provider=net.jini.loader.pref.RequireDlPermProvider
>
>
> OR
>
> java.rmi.server.RMIClassLoaderSpi=net.jini.loader.pref.RequireDlPermProvider
>
>
> If the first property is specified, it isn't required to be loaded by
> the system ClassLoader.
>
> The next step is to provide a service interface for a bootstrap proxy
> to return a smart proxy and to define a suitable Entry for the
> bootstrap proxy to declare the interfaces it's smart proxy implements.
>
> Clients can then lookup bootstrap proxy, based on the Entry,
> authenticate the bootstrap proxy and grant it DownloadPermission
> dynamically. The client will then need to prepare the smart proxy,
> placing constraints on it.
>
> While the bootstrap proxy should have an AtomicVerification.YES
> constraint, the endpoint for the smart proxy doesn't have to, so after
> trust has been established, it is possible to run with standard
> serialization. In this case the smart proxy itself must be
> serializable using @AtomicSerial, but objects serialized over the
> smart proxy's endpoint's need not. I've also have made it possible
> for proxy codebases to declare the permissions they require so these
> can be granted dynamically after trust is established.
>
> This would also provide the client with delayed unmarshalling
> functionality, while preserving the standard ServiceRegistrar
> interface, which can be managed using ServiceDiscoveryManager, and
> ServiceItemFilter.
>
> In other words, this additional functionality requires minimal effort
> on behalf of the developer, while those who don't need it can remain
> blissfully ignorant and implement it later if they want to.
>
> I haven't uploaded to svn, would you like to see some patches?
>
> Regards,
>
> Peter.
