In Item50, there was a range class of immutable dates with a mutable private Date field. It was protected from being immutable by using a defensive copy of the Date object in its constructor and accessor.
package tryAny.effectiveJava;
import java.io.Serializable;
import java.util.Date;
public final class Period implements Serializable {
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(this.start + " after " + this.end);
}
public Date start() {
return new Date(start.getTime());
}
public Date end() {
return new Date(end.getTime());
}
public String toString() {
return start + "-" + end;
}
}
Consider making this class Serializable. Since there is no discrepancy between the physical structure and the logical data content, it seems that it is sufficient to use the default serialized form, that is, to implement Serializable, but doing so does not keep the invariants.
The readObject method is essentially a public constructor, which is problematic in that it requires the same care as other constructors. The constructor needs to check the arguments (Item49) and requires a defensive copy of the arguments (Item50). Therefore, readObject has the same consideration. Forgetting these considerations in readObject allows an attacker to break invariants.
Roughly speaking, readObject is a constructor that treats a byte stream as a single argument. Bytestreams are usually the result of serializing a normally constructed instance. The problem is a byte stream artificially constructed to violate invariants. Such a byte stream results in the creation of objects that would not normally be possible. Below is an example (Period's end comes before start), but in my environment,
Exception in thread "main" java.lang.IllegalArgumentException: java.lang.ClassNotFoundException: Period
at tryAny.effectiveJava.BogusPeriod.deserialize(BogusPeriod.java:31)
at tryAny.effectiveJava.BogusPeriod.main(BogusPeriod.java:20)
Will come out. ..
package tryAny.effectiveJava;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.io.ObjectInputStream;
public class BogusPeriod {
// Byte stream could not have come from real Period instance
private static final byte[] serializedForm = new byte[] { (byte) 0xac, (byte) 0xed, 0x00, 0x05, 0x73, 0x72, 0x00,
0x06, 0x50, 0x65, 0x72, 0x69, 0x6f, 0x64, 0x40, 0x7e, (byte) 0xf8, 0x2b, 0x4f, 0x46, (byte) 0xc0,
(byte) 0xf4, 0x02, 0x00, 0x02, 0x4c, 0x00, 0x03, 0x65, 0x6e, 0x64, 0x74, 0x00, 0x10, 0x4c, 0x6a, 0x61, 0x76,
0x61, 0x2f, 0x75, 0x74, 0x69, 0x6c, 0x2f, 0x44, 0x61, 0x74, 0x65, 0x3b, 0x4c, 0x00, 0x05, 0x73, 0x74, 0x61,
0x72, 0x74, 0x71, 0x00, 0x7e, 0x00, 0x01, 0x78, 0x70, 0x73, 0x72, 0x00, 0x0e, 0x6a, 0x61, 0x76, 0x61, 0x2e,
0x75, 0x74, 0x69, 0x6c, 0x2e, 0x44, 0x61, 0x74, 0x65, 0x68, 0x6a, (byte) 0x81, 0x01, 0x4b, 0x59, 0x74, 0x19,
0x03, 0x00, 0x00, 0x78, 0x70, 0x77, 0x08, 0x00, 0x00, 0x00, 0x66, (byte) 0xdf, 0x6e, 0x1e, 0x00, 0x78, 0x73,
0x71, 0x00, 0x7e, 0x00, 0x03, 0x77, 0x08, 0x00, 0x00, 0x00, (byte) 0xd5, 0x17, 0x69, 0x22, 0x00, 0x78 };
public static void main(String[] args) {
Period p = (Period) deserialize(serializedForm);
System.out.println(p);
}
// Returns the object with the specified serialized form
public static Object deserialize(byte[] sf) {
try {
InputStream is = new ByteArrayInputStream(sf);
ObjectInputStream ois = new ObjectInputStream(is);
return ois.readObject();
} catch (Exception e) {
throw new IllegalArgumentException(e.toString());
}
}
}
The upper class should normally show the end date before the start date, which violates the invariants of the class.
In order to prevent this attack, it is necessary to perform validation check with readObject method as shown below.
private void readObject(ObjectInputStream s) throws IOException, ClassNotFoundException {
s.defaultReadObject();
if (start.compareTo(end) > 0) {
throw new InvalidObjectException(start + "after" + end);
}
}
Even if the above attack can be prevented, it is possible to create an instance while observing the conditions of Period, and add a reference to Date in the private field of Period at the end.
That is, an attacker reads a Period instance from an ObjectInputStream and reads the "bad object reference" attached to that stream. These references allow an attacker to obtain a reference to a Date instance within a Period object and modify the Period instance. The following is an example.
package tryAny.effectiveJava;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.util.Date;
public class MutablePeriod {
public final Period period;
public final Date start;
public final Date end;
public MutablePeriod() {
try {
ByteArrayOutputStream bos = new ByteArrayOutputStream();
ObjectOutputStream out = new ObjectOutputStream(bos);
//Write Period instance
out.writeObject(new Period(new Date(), new Date()));
//Create a reference to a specific property of your Period instance
byte[] ref = { 0x71, 0, 0x7e, 0, 5 };
bos.write(ref);
ref[4] = 4;
bos.write(ref);
//Generate variable Period instances
ObjectInputStream in = new ObjectInputStream(new ByteArrayInputStream(bos.toByteArray()));
period = (Period) in.readObject();
start = (Date) in.readObject();
end = (Date) in.readObject();
} catch (IOException | ClassNotFoundException e) {
throw new AssertionError(e);
}
}
public static void main(String[] args) {
MutablePeriod mp = new MutablePeriod();
Period p = mp.period;
Date pEnd = mp.end;
// Let's turn back the clock
pEnd.setYear(78);
System.out.println(p);
// Bring back the 60s!
pEnd.setYear(69);
System.out.println(p);
}
}
When this program is executed, the output is as follows.
Mon Oct 08 18:43:49 JST 2018-Sun Oct 08 18:43:49 JST 1978
Mon Oct 08 18:43:49 JST 2018-Wed Oct 08 18:43:49 JST 1969
If you leave the Period instance as it is, you will be free to manipulate the internal components. Systems that rely on Period's immutability for security can then be attacked.
The cause is that the defensive copy was not taken in the readObject method of Period. ** When an object is deserialized, it is imperative to make a defensive copy of any field that contains object references that the client should not keep. ** ** Specifically, it should be a readObject like the one below. (At this time, start and end are no longer final)
private void readObject(ObjectInputStream s) throws IOException, ClassNotFoundException {
s.defaultReadObject();
start = new Date(start.getTime());
end = new Date(end.getTime());
if (start.compareTo(end) > 0) {
throw new InvalidObjectException(start + "after" + end);
}
}
By doing this, even when MutablePeriod is executed, it will be as follows, and fraud will not occur.
Mon Oct 08 20:19:56 JST 2018-Mon Oct 08 20:19:56 JST 2018
Mon Oct 08 20:19:56 JST 2018-Mon Oct 08 20:19:56 JST 2018
You should ask if you want to create a public constructor that takes a non-transient field as an argument, and if you don't have to validate that argument. If that doesn't work, you'll also need to make a defensive copy of the readObject and perform a validation check. As an alternative, you should use the serialization proxy pattern (Item90).
Another similarity between readObject and the constructor is that you must not use methods that can be overridden, either directly or indirectly, inside the readObject method (Item19). If used, the overridden method will be called and the subclass state will be executed without being deserialized, which may result in an error.
The guidelines for writing readObject are summarized below.