SEC58-J. Deserialization methods should not perform potentially dangerous operations by default

A Serializable class can overload the Serializable.readObject() method, which is called when an object of that class is being deserialized.  Both this method and the method readResolve() should refrain from performing potentially dangerous operations.

This guideline complements rule SER12-J. Prevent deserialization of untrusted data.  Whereas SER12-J requires the programmer to ensure that data to be deserialized is trusted, this guideline requires that all serializable classes refrain, by default, from performing dangerous operations during deserialization.  This guideline is intended to address legacy code that must deserialize untrusted input, despite violating SER12-J.

For compliance with this guideline, it is permitted to assume that, if an ObjectInputStream contains a whitelist, then control will pass to the readObject() or readResolve() method of a class if and only if that class is on the whitelist.  In other words, a class does not need to check that it appears on the whitelist; it only needs to check that a whitelist exists.  This eliminates the need to perform a redundant check against the whitelist, and it enables compatibility with a greater range of whitelist implementations.

Non-Compliant Code Example

In the following non-compliant code example, the class OpenedFile opens a file during deserialization.  Operating systems typically impose a limit on the number of open file handles per process. Usually, this limit is not large (e.g., 1024).  Consequently, deserializing a list of OpenedFile objects can consume all file handles available to the process and consequently cause the program to malfunction if it attempts to open another file before the deserialized OpenedFile objects get garbage-collected.

import java.io.*;

class OpenedFile implements Serializable {
  String filename;
  BufferedReader reader;

  public OpenedFile(String filename) throws FileNotFoundException {
    this.filename = filename;
    init();
  }

  private void init() throws FileNotFoundException {
    reader = new BufferedReader(new FileReader(filename));
  }
    
  private void writeObject(ObjectOutputStream out) throws IOException {
    out.writeUTF(filename);
  }

  private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
    filename = in.readUTF();
    init();
  }
} 

Compliant Solution

In this compliant solution, the readObject() method throws an exception unless the deserialization is protected by a whitelist.  Note that this compliant solution is complementary to the compliant solution in SER12-J. Prevent deserialization of untrusted data.  In that compliant solution, the source code location that invokes deserialization is modified to use a custom subclass of ObjectInputStream which overrides the resolveClass() method to check whether the class of the serialized object is whitelisted before that class's readObject() method gets called.  In contrast, in the following compliant solution, the presence of a whitelist is checked inside the readObject() method of the dangerous serializable class.  We do not need to verify that the whitelist actually contains the class, because if it did not, the readObject() method would never get executed.

import java.io.*;
import java.lang.reflect.*;
 
class OpenedFile implements Serializable {
  String filename;
  BufferedReader reader;

  public OpenedFile(String filename) throws FileNotFoundException {
    this.filename = filename;
    init();
  }

  private void init() throws FileNotFoundException {
    reader = new BufferedReader(new FileReader(filename));
  }
     
  private void writeObject(ObjectOutputStream out) throws IOException {
    out.writeUTF(filename);
  }

  private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
    boolean hasWhitelist = false;
    try {
        in.getClass().getDeclaredField("whitelist");
        hasWhitelist = true;
    } catch (ReflectiveOperationException e) {}
    if (!hasWhitelist) {
      throw new SecurityException("Deserialization without a whitelist is disallowed for class " + 
                                  this.getClass().getName() + ".");
    }
    filename = in.readUTF();
    init();
  }
}

Compliant Solution

In this compliant solution, potentially dangerous operations are moved outside of deserialization, and users of the class are required to make a separate call to init() after deserializing.

import java.io.*;
 
class OpenedFile implements Serializable {
  String filename;
  BufferedReader reader;
  boolean isInitialized;

  public OpenedFile(String filename) {
    this.filename = filename;
    isInitialized = false;
 }

  public void init() throws FileNotFoundException {
    reader = new BufferedReader(new FileReader(filename));
    isInitialized = true;
 }
     
  private void writeObject(ObjectOutputStream out) throws IOException {
    out.writeUTF(filename);
  }

  private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
    filename = in.readUTF();
    isInitialized = false;
 }
}

Related Vulnerabilities

CERT Vulnerability #576313 describes a family of exploitable vulnerabilities that arise from violating this rule.

Risk Assessment

The severity of violations of this rule depend on the nature of the potentially dangerous operations performed.  If only mildly dangerous operations are performed, the risk might be limited to denial-of-service (DoS) attacks.  At the other extreme, remote code execution is possible if attacker-supplied input is supplied to methods such as Runtime.exec (either directly or via reflection).

Automated Detection

Related Guidelines

Bibliography