groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Wagenleitner <john.wagenleit...@gmail.com>
Subject Re: MethodClosure deserialization problem
Date Sun, 02 Oct 2016 00:57:11 GMT
On Sat, Oct 1, 2016 at 12:55 AM, Jochen Theodorou <blackdrag@gmx.org> wrote:

> hi all,
>
> as you guys may remember we added
>
> private Object readResolve() {
>>   if (ALLOW_RESOLVE) {
>>     return this;
>>   }
>>   throw new UnsupportedOperationException();
>> }
>>
>
> to prevent proper deserialization of a MethodClosure in case we don't want
> to allow it (which is the default). Now it seems that this approach is not
> enough. I have found several articles stating that it is possible to bypass
> readResolve. In this case you would still have access to the fully
> deserialized object. Thus I suggest we do the following:
>
> private void readObject(java.io.ObjectInputStream stream) throws
>> IOException, ClassNotFoundException {
>>   if (ALLOW_RESOLVE) {
>>     stream.defaultReadObject();
>>   } else {
>>     stream.readUTF(); // read method and forget it
>>   }
>> }
>>
>
>

Preventing the deserialization at this early stage before the object is
constructed I think is a good thing.  But I am curious, why try to read the
method String value and forget it?  Not sure if this is a concern, but even
though this will leave the method field null I think it would still try to
deserialize the fields for the super class Closure.  Seems like the safer
route to take her is to throw an exception as before.




> this is very similar to the readResolve implementation of course. So we
> would still fail deserialization, only much earlier. We would still read
> the String for the method, but we would make sure it will not be assigned.
>
> So if malicious code manage to go around readResolve, it would still be
> left with a MethodClosure in which method is null, thus any try to invoke a
> method will fail with a NullpointException.
>
> Afaik this solution would be compatible to earlier versions of Groovy.
>
> What do you guys think?
>
> bye Jochen
>

Mime
View raw message