Code review request for 6990094 "ObjectInputStream cloneArray doesn't handle short[]"

[Stuart Marks]

Hi Rémi,

I think instead of

         public Resolver(Object o) {
             this.replacement = replacement;
         }

that you meant

         public Resolver(Object o) {
             this.replacement = o;
         }

Correct?

Anyway, the technique you suggest does work to reproduce the bug, in that it 
causes Resolver.readResolve() to return a reference to the same object when it 
deserializes two different objects. The code does so by serializing the same 
object twice and pushing it through the object stream.

However, this is a fairly roundabout way of causing the bug to occur. All 
that's necessary is to make sure that Resolver.readResolve() returns the same 
reference twice. The easiest way to do this is with a static. In fact, I've 
suggested to Joe that he remove the setReplacement() method and just assign to 
a static in the top-level class, just prior to the calls to readUnshared(). 
That makes it clear that there's nothing tricky going on in during 
serialization, just during deserialization.

s'marks




On 12/6/10 9:15 AM, Rémi Forax wrote:
> On 12/06/2010 05:03 PM, Peter Jones wrote:
>> On Mon, Dec 6, 2010 at 2:35 AM, Joe Darcy<joe.darcy at oracle.com> wrote:
>>> Off-list, Alan found the a related closed test and Stuart and I have
>>> developed an explicit test that tickles this bug:
>>>
>>> http://cr.openjdk.java.net/~darcy/6990094.1/
>> Looks good to me.
>>
>> On Mon, Dec 6, 2010 at 3:10 AM, Rémi Forax<forax at univ-mlv.fr> wrote:
>>> Hi Joe,
>>> In the test, I don't see why the replacement field has to be static in
>>> Resolver.
>>> In my opinion, a private final field is sufficient.
>> I don't know on what instance you would set such an instance field, to
>> control the exact reference returned by invoking readUnshared on a
>> deserialized instance. The attack scenario addressed by the original
>> bug fix would likely use a static field similarly.
>>
>> -- Peter
>
> I mean create two instances sharing the same replacement Object.
>
> static class Resolver implements Serializable {
> private final Object replacement;
>
> public Resolver(Object o) {
> this.replacement = replacement;
> }
> private Object readResolve() throws ObjectStreamException {
> return replacement;
> }
> }
>
> private static void test(Object replacement)
> throws IOException, ClassNotFoundException {
>
> try(ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
> try(ObjectOutputStream oos = new ObjectOutputStream(baos)) {
> oos.writeObject(new Resolver(replacement));
> oos.writeObject(new Resolver(replacement));
> }
>
> Rémi