Thursday, July 01, 2010

Why Complex+Powerful is a bad combination for security

(or: the big, ugly mess that is Java serialization)

I decided to write this bit after I started participating on the Cert/CC Java Secure Coding guidelines and was looking at the rules in the Serialization section. I immediately spotted some problems and started working on how to do serialization right, in terms of security. I already knew many of the pitfalls, but I quickly found that secure validation while deserializing is extremely difficult. Need proof? If Joshua Bloch can't get it right, and the (Oracle/Sun) Secure Coding Guidelines can't get it right, and key core classes can't get it right, what chances do the rest of us have?

Java deserialization privilege escalation vulnerabilities keep popping up. I know of three that have been fixed, and it's a safe bet we haven't seen the last of them. But they've been discussed at length, so let's look at the rest of the problems in the world of Java serialization.

Early References - It is possible to create a cross reference scenario where any object controlled by the attacker gets a reference to another, sensitive object which is still being initialized. The attacker creates serialized data where an instance of the class SensitiveClass references an instance of EvilClass. EvilClass has a reference of that same SensitiveClass instance. Thus, when SensitiveClass is in the middle of being serialized, in order to deserialize it's fields, the EvilClass instance gets deserialized. If EvilClass has a readObject method, it gets control and a reference to the SensitiveClass which is still under construction.

Phantom Fields - Phantom Fields are an auxiliary technique to obtain early, incomplete references. A class such as integer does not have any object fields, so the early reference attack would not work by itself. But we can create n fields in the serialized data that don't exist in the actual classes. The values for these get read and then discarded, but if their values are objects controlled by the attacker, they can have a readObject method which gets executed before the other object has been fully initialized.

Phantom Superclasses - These work much like phantom fields, but with different timing. They are processed prior to the subclasses. This is significant because primitive fields are processed before object fields (in other words: superclass primitives, superclass objects, subclass primitives, subclass objects, ...) . So if, for example, one wishes to create a mutable java.lang.Integer, it is necessary to create a phantom superclass, which has a field containing a reference to the Integer being deserialized.

Repeated Fields - In the serialized data, a valid field for a class can be repeated any number of times. The values of this field are then written to the field repeatedly, even if the field is final (through the magic of java.misc.Unsafe). Attacker can achieve mutability (TOCTOU) for seemingly immutable classes (java.lang.Integer, etc).

Race conditions - ObjectInputStream and ObjectOutputStream are not internally synchronized and a lot of the de/serialization logic relies on fields of these two classes, which are shared between threads.

External call to defaultReadObject - As the attacker has a reference to the ObjectInputStream, he can call the defaultReadObject method on objects that do not wish to use default deserialization. Due to the race condition problems, it is possible to force a defaultReadObject before the class manages to call getFields.

External call to defaultWriteObject - Much like external call to defaultReadObject, leveraging the race condition, it is possible to call defaultWriteObject externally from another thread for any object that has a writeObject method.


Now let's break some stuff:

1) Example from Joshua Bloch's Effective Java (Item 76):

It shows an example of an immutable date range class, with validation in the readObject method.

001 package ser1;
002 
003 import java.io.IOException;
004 import java.io.InvalidObjectException;
005 import java.io.ObjectInputStream;
006 import java.io.Serializable;
007 import java.util.Date;
008 
009 // Immutable class that uses defensive copying
010 public final class Period implements Serializable {
011 
012   private Date start;
013   private Date end;
014   /*
015    * @param start the beginning of the period
016    * @param end the end of the period; must not precede start
017    * @throws IllegalArgumentException if start is after end
018    * @throws NullPointerException if start or end is null
019    */
020   public Period(Date start, Date end) {
021     this.start = new Date(start.getTime());
022     this.end = new Date(end.getTime());
023     if (this.start.compareTo(this.end) > 0) throw new IllegalArgumentException(start + " after " + end);
024   }
025 
026   public Date start () {
027     return new Date(start.getTime());
028   }
029  
030   public Date end () {
031     return new Date(end.getTime());
032   }
033 
034   public String toString() {
035     return start + " - " + end;
036   }
037 
038   // readObject method with defensive copying and validity checking
039   private void readObject(ObjectInputStream s) throws IOException, ClassNotFoundException {
040     s.defaultReadObject();
041     // Defensively copy our mutable components
042     start = new Date(start.getTime());
043     end = new Date(end.getTime());
044     // Check that our invariants are satisfied
045     if (start.compareTo(end) > 0)
046       throw new InvalidObjectException(start + " after " + end);
047   }
048 
049   // Remainder omitted
050 }


Broken. This can be circumvented in a rather controlled fashion, using cross referencing and a Date subclass (MutableDate). Imagine that the serialized data contains a Period Object whose start and end fields contain mutable Date subclasses which contain a reference back to the Period object. Deserialization executes the readObject method and calls MutableDate.getTime(). At this moment, MutableDate contains a reference to the PeriodObject before the defensive copying takes place.


2) Oracle Secure Coding guidelines (Guideline 5-4):

2a) Example 1

001 package ser2;
002 
003 public final class SensitiveClass implements java.io.Serializable {
004 
005     public SensitiveClass() {
006         // permission needed to instantiate SensitiveClass
007         securityManagerCheck();
008 
009         // regular logic follows
010     }
011 
012     // implement readObject to enforce checks during deserialization
013     private void readObject(java.io.ObjectInputStream in) {
014         // duplicate check from constructor
015         securityManagerCheck();
016 
017         // regular logic follows
018     }
019 
020   private void securityManagerCheck() {
021     // code omitted
022   }
023 
024 }
025 


Broken. It is possible to call ObjectInputStream.defaultReadObject from another thread before the securityManagerCheck takes place (or during it).

2b) Example 2

001 package ser3;
002 
003 import java.io.IOException;
004 
005 public final class SecureName implements java.io.Serializable {
006 
007     // private internal state
008     private String name;
009 
010     private static final String DEFAULT = "DEFAULT";
011 
012     public SecureName() {
013         // initialize name to default value
014         name = DEFAULT;
015     }
016 
017     // allow callers to modify private internal state
018     public void setName(String name) {
019         if (name != null ? name.equals(this.name) : (this.name == null)) {
020             // no change - do nothing
021             return;
022         } else {
023             // permission needed to modify name
024             securityManagerCheck();
025 
026             inputValidation(name);
027 
028             this.name = name;
029         }
030     }
031 
032   // implement readObject to enforce checks during deserialization
033     private void readObject(java.io.ObjectInputStream in) throws ClassNotFoundException, IOException {
034         java.io.ObjectInputStream.GetField fields = in.readFields();
035         String name = (String) fields.get("name", DEFAULT);
036 
037         // if the deserialized name does not match the default value normally
038         // created at construction time, duplicate checks
039 
040         if (!DEFAULT.equals(name)) {
041             securityManagerCheck();
042             inputValidation(name);
043         }
044         this.name = name;
045     }
046 
047     private void inputValidation(String name2) {
048         // code omitted
049     }
050 
051     private void securityManagerCheck() {
052         // code omitted
053     }
054 
055 }


Broken. It is possible to call ObjectInputStream.defaultReadObject from another thread before the readObject method invokes the .readFields() method.


2c) Example 3 - Secure writeObject implementation with a security check.

001 package ser4;
002 
003 import java.io.IOException;
004 
005 public final class SecureValue implements java.io.Serializable {
006 
007     // sensitive internal state
008     private String value;
009 
010     // public method to allow callers to retrieve internal state
011     public String getValue() {
012         // permission needed to get value
013         securityManagerCheck();
014         return value;
015     }
016 
017     // implement writeObject to enforce checks during serialization
018     private void writeObject(java.io.ObjectOutputStream out) throws IOException {
019         // duplicate check from getValue()
020         securityManagerCheck();
021         out.writeObject(value);
022     }
023 
024     private void securityManagerCheck() {
025         // code omitted
026     }
027 }


Broken. It is possible to call defaultWriteObject externally before the writeObject method calls securityManagerCheck (or during it).


3) java.lang.Integer


...
037 public final class Integer extends Number implements Comparable<Integer> {
...
628     /**
629      * The value of the <code>Integer</code>.
630      *
631      * @serial
632      */
633     private final int value;
...
1202 }


Broken. With a Phantom Superclass and Repeated fields it is possible to create a mutable Integer. An Integer which is zero and later on changes it's value to something else can be done in a controlled fashion. An Integer which keeps changing it's value in arbitrary manner can be timed.

4) java.io.File


...
120 public class File
121     implements Serializable, Comparable<File>
122 {
...
129     /**
130      * This abstract pathname's normalized pathname string. A normalized
131      * pathname string uses the default name-separator character and does not
132      * contain any duplicate or redundant separators.
133      *
134      * @serial
135      */
136     private String path;
...
1918     /**
1919      * readObject is called to restore this filename.
1920      * The original separator character is read. If it is different
1921      * than the separator character on this system, then the old separator
1922      * is replaced by the local separator.
1923      */
1924     private synchronized void readObject(java.io.ObjectInputStream s)
1925          throws IOException, ClassNotFoundException {
1926         ObjectInputStream.GetField fields = s.readFields();
1927         String pathField = (String)fields.get("path", null);
1928         char sep = s.readChar(); // read the previous separator char
1929         if (sep != separatorChar)
1930             pathField = pathField.replace(sep, separatorChar);
1931         this.path = fs.normalize(pathField);
1932         this.prefixLength = fs.prefixLength(this.path);
1933     }
1934 
1935     /** use serialVersionUID from JDK 1.0.2 for interoperability */
1936     private static final long serialVersionUID = 301077366599181567L;
1937 }


Broken. It is possible to call ObjectInputStream.defaultReadObject externally along with a combination of phantom superclass and repeated field.

Conclusion

I'll put my code where my mouth is in future posts. The serialization framework is extremely complex and extremely powerful in that creates objects without calling constructors and sets values to fields that might be private and or final. This combination is a dangerous one for security.

3 comments:

jduck said...

Very nice post Sami. This does indeed appear very tricky. As usual, complexity causes human error, often with security implications.

I, and I suspect many others, are looking forward to your future work :)

Sami Koivu said...

Thanks Joshua :) Agreed.

swankjesse said...

Fantastic, illuminating post.