FIO31-J. Defensively copy mutable inputs and mutable internal components

A mutable input has the characteristic that its value may change between different accesses. This opens a window of opportunity for exploiting race conditions. A time-of-check, time-of-use (TOCTOU) inconsistency results when a field contains a value that passes the initial validation and security checks but mutates to a different value during actual use.

Additionally, an object's state may get corrupted if it returns references to internal mutable components. Accessors must consequently return defensive copies of internal mutable objects. (OBJ37-J. Defensively copy private mutable class members before returning their references)

Noncompliant Code Example

A TOCTOU inconsistency exists in this noncompliant code example. As cookie is a mutable input, an attacker may cause the cookie to expire between the initial check and the actual use.

public final class MutableDemo {
  // java.net.HttpCookie is mutable
  public void useMutableInput(HttpCookie cookie) {
    if (cookie == null) {
       throw new NullPointerException();
    }

    // Check if cookie has expired
    if(cookie.hasExpired()) {
      // Cookie is no longer valid, handle condition
    }

    // Cookie may have expired since time of check resulting in
    // an exception
    doLogic(cookie);
  }
}

Compliant Solution

The problem is alleviated by creating a copy of the mutable input and using it to perform operations so that the original object is left unscathed. This can be realized by implementing the java.lang.Cloneable interface and declaring a public clone method or by using a copy constructor. Performing a manual copy of object state within the caller becomes necessary if the mutable class is declared final (that is, it cannot provide an accessible copy method). See the guideline OBJ36-J. Provide mutable classes with a clone method to allow passing instances to untrusted code safely for more information. Note that any input validation must be performed on the copy and not the original object.

public final class MutableDemo {
  // java.net.HttpCookie is mutable
  public void useMutableInput(HttpCookie cookie) {
    if (cookie == null) {
      throw new NullPointerException();
    }

    // Create copy
    cookie = (HttpCookie)cookie.clone();

    // Check if cookie has expired
    if(cookie.hasExpired()) {
      // Cookie is no longer valid, handle condition
    }

    doLogic(cookie);
  }
}

Compliant Solution

Sometimes, the copy constructor or the clone() method returns a shallow copy of the original instance. For example, invocation of clone() on an array results in creation of an array instance that shares references to the same elements as the original instance. However, a deep copy that involves element duplication is required when the input consists of mutable components, such as an array of cookies. This compliant solution exemplifies this condition.

  public void deepCopy(int[] ints, HttpCookie[] cookies) {
    if (ints == null || cookies == null) {
      throw new NullPointerException();
    }

    // Shallow copy
    int[] intsCopy = ints.clone();

    // Deep copy
    HttpCookie[] cookiesCopy = new HttpCookie[cookies.length];
    for (int i = 0; i < cookies.length; i++) {
      // Manually create copy of each element in array
      cookiesCopy[i] = (HttpCookie)cookies[i].clone();
    }
 
    doLogic(intsCopy, cookiesCopy);
}

Noncompliant Code Example

When the mutable input type is non-final, a malicious subclass may override the clone() method. This is a serious issue unless the non-final input defends against it. This noncompliant code example demonstrates this weakness.

// java.util.ArrayList is mutable and non-final
public void copyNonFinalInput(ArrayList list) {
  doLogic(list);
}

Compliant Solution

To copy mutable inputs having a non-final type, create a new instance of the ArrayList. This instance can be forwarded to any code capable of modifying it.

// java.util.ArrayList is mutable and non-final
public void copyNonFinalInput(ArrayList list) {
  // Create new instance of declared input type 
  list = new ArrayList(list);
  doLogic(list);
}

Noncompliant Code Example

This noncompliant code example uses the Collection interface as an input parameter and directly passes it to doLogic().

// java.util.Collection is an interface
public void copyInterfaceInput(Collection<String> collection) {
  doLogic(collection);
}

Compliant Solution

This compliant solution instantiates a new ArrayList and forwards it to the doLogic() method.

public void copyInterfaceInput(Collection<String> collection) {
  // Convert input to trusted implementation
  collection = new ArrayList(collection);
  doLogic(collection);
}

Some objects appear to be immutable because they have no mutator methods. For example, the java.lang.CharacterSequence interface describes an immutable sequence of characters. It should be noted that if the underlying implementation on which the CharacterSequence is based changes, the value of the CharacterSequence also changes. Such objects must be defensively copied before use. It is also permissible to use the toString() method to make them immutable before passing them as parameters. Mutable fields must always never be stored in static variables. To avoid exposing mutable fields by storing them in static variables, creating defensive copies of the fields is highly recommended.

Risk Assessment

Failing to create a copy of a mutable input may enable an attacker to exploit a TOCTOU vulnerability and at other times, expose internal mutable components to untrusted code.

Automated Detection

TODO

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

References

[[SCG 07]] Guideline 2-1 Create a copy of mutable inputs and outputs
[[Bloch 08]] Item 39: Make defensive copies when needed
[[Pugh 09]] Returning references to internal mutable state

FIO30-J. Do not log sensitive information      09. Input Output (FIO)      FIO32-J. Ensure all resources are properly closed when they are no longer needed