Question

There are two attacks documented in readObject() methods, one is called BogusPeriod, and the other is called MutablePeriod. Implement either (your choice) of these attacks (basically involves typing in code) and verify that the attack takes place.

Answer 1

Item 39 contains an immutable date-range class containing mutable private Date fields. The class goes to great lengths to preserve its invariants and its immutability by defensively copying Date objects in its constructor and accessors. Here is the class:

// Immutable class that uses defensive copying

public final class Period {

private final Date start;

private final Date end;

/**

* @param start the beginning of the period

* @param end the end of the period; must not precede start

* @throws IllegalArgumentException if start is after end

* @throws NullPointerException if start or end is null

*/

public Period(Date start, Date end) {

this.start = new Date(start.getTime());

this.end = new Date(end.getTime());

if (this.start.compareTo(this.end) > 0)

throw new IllegalArgumentException(

start + " after " + end);

}

public Date start () { return new Date(start.getTime()); }

public Date end () { return new Date(end.getTime()); }

public String toString() { return start + " - " + end; }

... // Remainder omitted

}

Suppose you decide that you want this class to be serializable. Because the physical representation of aPeriod object exactly mirrors its logical data content, it is not unreasonable to use the default serialized form (Item 75). Therefore, it might seem that all you have to do to make the class serializable is to add the words “implements Serializable” to the class declaration. If you did so, however, the class would no longer guarantee its critical invariants.

The problem is that the readObject method is effectively another public constructor, and it demands all of the same care as any other constructor. Just as a constructor must check its arguments for validity (Item 38) and make defensive copies of parameters where appropriate (Item 39), so must a readObject method. If areadObject method fails to do either of these things, it is a relatively simple matter for an attacker to violate the class’s invariants.

Loosely speaking, readObject is a constructor that takes a byte stream as its sole parameter. In normal use, the byte stream is generated by serializing a normally constructed instance. The problem arises whenreadObject is presented with a byte stream that is artificially constructed to generate an object that violates the invariants of its class. To fix this problem, provide a readObject method for Period that callsdefaultReadObject and then checks the validity of the deserialized object. If the validity check fails, thereadObject method throws an InvalidObjectException, preventing the deserialization from completing:

// readObject method with validity checking

private void readObject(ObjectInputStream s)

throws IOException, ClassNotFoundException {

s.defaultReadObject();

// Check that our invariants are satisfied

if (start.compareTo(end) > 0)

throw new InvalidObjectException(start +" after "+ end);

}

While this fix prevents an attacker from creating an invalid Period instance, there is a more subtle problem still lurking. It is possible to create a mutable Period instance by fabricating a byte stream that begins with a valid Period instance and then appends extra references to the private Date fields internal to the Periodinstance.

The source of the problem is that Period’s readObject method is not doing enough defensive copying. When an object is deserialized, it is critical to defensively copy any field containing an object reference that a client must not possess. Therefore, every serializable immutable class containing private mutable components must defensively copy these components in its readObject method. The following readObjectmethod suffices to ensure Period’s invariants and to maintain its immutability:

// readObject method with defensive copying and validity checking

private void readObject(ObjectInputStream s)

throws IOException, ClassNotFoundException {

s.defaultReadObject();

// Defensively copy our mutable components

start = new Date(start.getTime());

end = new Date(end.getTime());

// Check that our invariants are satisfied

if (start.compareTo(end) > 0)

throw new InvalidObjectException(start +" after "+ end);

}

Note that the defensive copy is performed prior to the validity check and that we did not use Date’s clonemethod to perform the defensive copy. Both of these details are required to protect Period against attack (Item 39). Note also that defensive copying is not possible for final fields. To use the readObject method, we must make the start and end fields nonfinal.

Here is a simple litmus test for deciding whether the default readObject method is acceptable for a class: would you feel comfortable adding a public constructor that took as parameters the values for each nontransient field in the object and stored the values in the fields with no validation whatsoever? If not, you must provide a readObject method, and it must perform all the validity checking and defensive copying that would be required of a constructor. Alternatively, you can use the serialization proxy pattern (Item 78).

There is one other similarity between readObject methods and constructors, concerning nonfinal serializable classes. A readObject method must not invoke an overridable method, directly or indirectly (Item 17). If this rule is violated and the method is overridden, the overriding method will run before the subclass’s state has been deserialized. A program failure is likely to result.

To summarize, anytime you write a readObject method, adopt the mind-set that you are writing a public constructor that must produce a valid instance regardless of what byte stream it is given. Do not assume that the byte stream represents an actual serialized instance. While the examples in this item concern a class that uses the default serialized form, all of the issues that were raised apply equally to classes with custom serialized forms. Here, in summary form, are the guidelines for writing a bulletproof readObject method:

• For classes with object reference fields that must remain private, defensively copy each object in such a field. Mutable components of immutable classes fall into this category.

• Check any invariants and throw an InvalidObjectException if a check fails. The checks should follow any defensive copying.

• If an entire object graph must be validated after it is deserialized, use the ObjectInputValidation interface [JavaSE6, Serialization].

• Do not invoke any overridable methods in the class, directly or indirectly.

This noncompliant code example fails to defensively copy the mutable Date object date. An attacker might be able to create an instance of MutableSer whose date object contains a nefarious subclass of Date and whose methods can perform actions specified by an attacker. Any code that depends on the immutability of the subobject is vulnerable.

class MutableSer implements Serializable {   private static final Date epoch = new Date(0);   private Date date = null; // Mutable component       public MutableSer(Date d){     date = new Date(d.getTime()); // Constructor performs defensive copying   }   private void readObject(ObjectInputStream ois) throws IOException, ClassNotFoundException {     ois.defaultReadObject();     // Perform validation if necessary   } }

Compliant Solution

This compliant solution creates a defensive copy of the mutable Date object date in the readObject() method. Note the use of field-by-field input and validation of incoming fields. Additionally, note that this compliant solution is insufficient to protect sensitive data (see SER03-J. Do not serialize unencrypted sensitive data for additional information).

private void readObject(ObjectInputStream ois) throws IOException, ClassNotFoundException {   ObjectInputStream.GetField fields = ois.readFields();   Date inDate = (Date) fields.get("date", epoch);   // Defensively copy the mutable component   date = new Date(inDate.getTime());   // Perform validation if necessary }

There is no need to copy immutable subobjects. Also, avoid using the subobject's clone() method because it can be overridden when the subobject's class is not final and produces only a shallow copy. The references to the subobjects themselves must be nonfinal so that defensive copying can occur. It is also inadvisable to use the writeUnshared() and readUnshared() methods as an alternative

One could craft serialized data where the object stored in the date field of the MutableSer class is a Date subclass, MutableDate, which holds a reference to the MutableSer instance (so MutableSer references MutableDate which references MutableSer). When the readObject method calls getTime() on the MutableDate class, it has a fully functional instance of the MutableSer class, with itself (a mutable date) in the date field.

As an example of how to create such serialized data and the auxiliary MutableDate:

public class CreateEvilSerializedData {   public static void main(String[] args) throws Exception {     MutableSer ser = new MutableSer(new Date());               MutableDate mutable = new MutableDate();     mutable.captured = ser;     // reflection to set a desired value     Field field = MutableSer.class.getDeclaredField("date");     field.setAccessible(true);     field.set(ser, mutable);               FileOutputStream fos = new FileOutputStream("ser07.ser");     ObjectOutputStream oos = new ObjectOutputStream(fos);     oos.writeObject(ser);     oos.close();   }   static class MutableDate extends Date {     private static final long serialVersionUID = 1L;     Object captured;     public long getTime() {       // at this point we have a MutableSer object that's initialized and has       // a MutableDate as the date       System.out.println(captured);       super.setTime(0);       System.out.println(captured);       return super.getTime();     }   } }