Java classes and methods may have invariants. An invariant is a property that is assumed to be true at certain points during program execution but is not formally specified as true. Invariants may be used in assert statements or may be informally specified in comments.

Method invariants can include guarantees made about what the method can do, requirements about the state of the object when the method is invoked, or guarantees about the state of the object when the method completes. For example, a method of a Date class might guarantee that 1 <= day_of_month <= 31 when the method exits.  

Class invariants are guarantees made about the state of their objects' fields upon the completion of any of their methods. For example, classes whose member fields may not be modified once they have assumed a value are called immutable classes. An important consequence of immutability is that the invariants of instances of these classes are preserved throughout their lifetimes.

Similarly, classes can rely on invariants to properly implement their public interfaces. These invariants might relate to the state of member fields or the implementation of member methods. Generally, classes can rely on encapsulation to help maintain these invariants, for example, by making member fields private. However, encapsulation can be incompatible with extensibility. For example, a class designer might want a method to be publicly accessible yet rely on the particulars of its implementation when using it in another method within the class. In this case, overriding the method in a subclass can break the internal invariants of the class.  Extensibility consequently carries with it two significant risks: a subclass can fail to satisfy the invariants promised to clients by its superclass, and it can break the internal invariants on which the superclass relies. For example, an immutable class that lacks the final qualifier can be extended by a malicious subclass that can modify the state of the supposedly immutable object. Furthermore, a malicious subclass object can impersonate the immutable object while actually remaining mutable. Such malicious subclasses can violate program invariants on which clients depend, consequently introducing security vulnerabilities. Note that the same can be said for a benign subclass that mistakenly supports mutability. These risks relate to both benign and malicious development.

To mitigate these risks, by default classes should be declared final unless there is a definite need for the class to be extensible. In that case, developers must carefully design the class with extensibility in mind. As a specific instance of this recommendation, classes that are designed to be treated as immutable either must be declared final or must have all of their member methods and fields declared final or private.

In systems where code can come from mixed protection domains, some superclasses might want to permit extension by trusted subclasses while simultaneously preventing extension by untrusted code. Declaring such superclasses to be final is infeasible because it would prevent the required extension by trusted code. One commonly suggested approach is to place code at each point where the superclass can be instantiated to check that the class being instantiated is either the superclass itself or a trustworthy subclass. However, this approach is brittle and is safe only in Java SE 6 or higher (see OBJ11-J. Be wary of letting constructors throw exceptions for a full discussion of the issues involved).

Noncompliant Code Example (BigInteger)

The java.math.BigInteger class is itself an example of noncompliant code. It is nonfinal and consequently extendable, which can be a problem when operating on an instance of BigInteger that was obtained from an untrusted client. For example, a malicious client could construct a spurious mutable BigInteger instance by overriding BigInteger's member functions [Bloch 2008].

The following code example demonstrates such an attack:

// Malicious subclassing of java.math.BigInteger
class BigInteger extends java.math.BigInteger {
  private byte[] ba;

  public BigInteger(String str) {
    super(str);
    ba = (new java.math.BigInteger(str)).toByteArray();
  }

  public BigInteger(byte[] ba) {
    super(ba);
    this.ba = ba;
  }

  public void setValue(byte[] ba) {
    this.ba = ba;
  }

  @Override public byte[] toByteArray() {
    return ba;
  }
}

Unlike the benign BigInteger class, this malicious BigInteger class is clearly mutable because of the setValue() method. Any code that receives an object of this class and assumes that the object is immutable will behave unexpectedly, as shown by the following code:

BigInteger bi = new BigInteger("123");

bi.setValue((new BigInteger("246")).toByteArray());

System.out.println(new BigInteger(bi.toByteArray()));  

This code prints: "246", which shows that the value of the supposedly immutable BigInteger bi has been changed.

OBJ01-J. Limit accessibility of fields points out that invariants cannot be enforced for mutable objects. TSM03-J. Do not publish partially initialized objects describes object construction and visibility issues specific to mutable objects, and CON50-J. Do not assume that declaring a reference volatile guarantees safe publication of the members of the referenced object and CON52-J. Document thread-safety and use annotations where applicable discuss some concurrency issues associated with mutable objects.

Violation of this recommendation can be mitigated by treating objects from untrusted sources as potentially malicious subclasses, as directed by OBJ06-J. Defensively copy mutable inputs and mutable internal components. Complying with that rule protects you from the consequences of violating this recommendation.

This example is particularly important because the BigInteger type has several useful cryptographic applications.

Noncompliant Code Example (Security Manager)

This noncompliant code example proposes adding a security manager check in the constructor of the java.math.BigInteger class. The security manager denies access when it detects that a subclass without the requisite permissions is attempting to instantiate the superclass [SCG 2009]. It also compares class types, in compliance with OBJ09-J. Compare classes and not class names. Note that this check does not prevent malicious extensions of BigInteger; it instead prevents the creation of BigInteger objects from untrusted code, which also prevents creation of objects of malicious extensions of BigInteger.

package java.math;
 
// ...
 
public class BigInteger {
  public BigInteger(String str) {
    securityManagerCheck(); 

    // ...
  }

  // Check the permission needed to subclass BigInteger
  // throws a security exception if not allowed
  private void securityManagerCheck() {
    // ...
  }
}

Unfortunately, throwing an exception from the constructor of a nonfinal class is insecure because it allows a finalizer attack (see OBJ11-J. Be wary of letting constructors throw exceptions). Furthermore, since BigInteger is Serializable, an attacker could bypass the security check by deserializing a malicious instance of BigInteger. For more information on proper deserialization, see the rule SER04-J. Do not allow serialization and deserialization to bypass the security manager.

Compliant Solution (Final)

This compliant solution prevents creation of malicious subclasses by declaring the immutable java.math.BigInteger class to be final. Although this solution would be appropriate for locally maintained code, it cannot be used in the case of java.math.BigInteger because it would require changing the Java SE API, which has already been published and must remain compatible with previous versions.

package java.math;

// ...

final class BigInteger {
  // ...
}

Compliant Solution (Java SE 6, Public and Private Constructors)

This compliant solution invokes a security manager check as a side effect of computing the Boolean value passed to a private constructor (as seen in OBJ11-J. Be wary of letting constructors throw exceptions). The rules for order of evaluation require that the security manager check must execute before invocation of the private constructor. Consequently, the security manager check also executes before invocation of any superclass's constructor.

This solution prevents the finalizer attack; it applies to Java SE 6 and later versions, where throwing an exception before the java.lang.Object constructor exits prevents execution of finalizers [SCG 2009].

package java.math;

// ...

public class BigInteger {
  public BigInteger(String str) {
    this(str, check());
  }

  private BigInteger(String str, boolean dummy) {
    // Regular construction goes here
  }

  private static boolean check() {
    securityManagerCheck(); 
    return true;
  }
}

Automated Detection

This recommendation is not checkable because it depends on factors that are unspecified in the code, including the invariants upon which the code relies and the necessity of designating a class as extensible, among others. However, simple statistical methods might be useful to find codebases that violate this recommendation by checking whether a given codebase contains a higher-than-average number of classes left nonfinal.

Related Guidelines

Secure Coding Guidelines for Java SE, Version 5.0

Guideline 4-5 / EXTEND-5: Limit the extensibility of classes and methods

Bibliography

[API 2006]

Class BigInteger

[Bloch 2008]

Item 15: "Minimize mutability"

Item 17, "Design and Document for Inheritance or Else Prohibit It"

[Gong 2003]

Chapter 6, "Enforcing Security Policy"

[Lai 2008]

Java Insecurity, Accounting for Subtleties That Can Compromise Code

[McGraw 1999]

Chapter 7, Rule 3, Make everything final, unless there's a good reason not to

[SCG 2009]Guideline 4-5 / EXTEND-5: Limit the extensibility of classes and methods
[Ware 2008] 

 


18 Comments

  1. In Java, the class of an object never changes, even during construction. "Virtual" methods may be called freely even before construction has finished. For instance Swing component constructors often call create methods, which need to be overridden with care.

    The important part of Guideline 1-2 is the first paragraph. Make sensitive classes non-subclassable (private/package-private constructor or final class). Other techniques are tricky to get right, are more verbose and mainly result from having to maintain an API.

    It is probably worth noting that a subclass with a finalizer can resurrect itself even with the compliant solution.

  2. setAccessible has a security manager check, and will therefore fail (unless the attack code has full permissions anyway). Reflection shouldn't give you any more permissions than calling direct (in fact, it can give you less).

  3. How is this solution supposed to work? Is the public ctor of BigInteger not hardcoding the argument passed to private static boolean check(Class); method to BigInteger.class? If I construct a MaliciousBigInteger that subclasses BigInteger, the argument that will be passed to BigInteger.check() will still be BigInteger.class and invocation of securityManagerCheck() will therefore be skipped...

    1. I've revamped the BigInteger-related compliant solutions. The class check was useless. The job of preventing malicious BigInteger objects comes from the securityManagerCheck() function, which presumably only enables trusted code to create BigInteger objects. Preventing any non-final class from being extended by all code in a constructor is impossible, because at construction time you don't know if your benign class is actually being extended by a malicious class. I suppose you could examine the stack to see who invoked the constructor. But that is the specialty of security managers, so we let them handle that work.

      1. I am not sure if the example is relevant anymore with respect to the requirement stated in the guideline that "Some superclasses must permit extension by trusted subclasses while simultaneously preventing extension by untrusted code". The example is actually more restrictive towards untrusted code in that it not only allows subclassing but also prevents untrusted code from using legitimate BigInteger objects. If on the other hand you grant untrusted code the permission to create BigInteger objects, then you are still vulnerable to malicious subclassing...

        1. I'll concede that last CS is not actually true to the letter of the law, as expressed in this rule, but does obey the spirit. Basic Principles:

          • Unless you declare your class final, any code that can access your class can subclass it.
          • Throwing exceptions in constructors is problematic (for reasons explained in the rule), and the compliant solutions mitigate this pitfall.
          • Adding security checks in member functions (rather than constructors) don't prevent malicious subclasse creation, but can prevent the malicious subclass from being mistaken for the (benign) superclass. Such checks are brittle and hinder performance, though.

          The problem explained by this rule is really two problems: (1) how to prevent malicious developers from accessing sensitive parts of your code via malicious subclasses, and (2) how to prevent incompetent but benign developers from creating subclasses that break your class invariants (eg the BigInteger.setValue() method).

          1. I have significantly edited this rule to make it more focused. I extended the discussion of invariants, shortened the discussion of enforcing trusted subclassing, and dropped what I saw as two orthogonal issues: methods receiving non-final classes as inputs and use of overridable methods in privileged blocks. While both of these issues are related, they are rather separate normative requirements and do not belong in this rule. Moreover, they are redundant with other rules.

            I also generalized the title to emphasize the more general nature of the normative component.

  4. Some thoughts on the title:  OBJ00-J. Limit extensibility of classes and methods with invariants

    English-wise, shouldn't this say "Limit the extensibility..."

    Also, there is no way to limit the extensibility of methods; only classes, so:

    "Limit the extensibility of classes with class or method invariants"

    Or is it sufficient to say:

    "Limit the extensibility of classes with invariants"

  5. This phrase "To mitigate these risks, classes must be declared final by default. " makes it sound like the rule requires this, while the next line makes it clear it does not.

    1. Replaced "must" with "should" and reworded.

      1. Thanks.  Did David talk to you about the first NCE?

        1. The first NCCE has been completely revised.  Please review.

  6. The java.math.BigInteger class is itself an example of noncompliant code.

    • A very bold statement to make, I'm not sure we should go this far. I suspect most Java library classes that are not final could be subclassed by malicious attackers. Are we prepared to declare BigInteger (along with most Java library classes) a violation of our standard? We may need to demote this to a recommendation.
    • The first NCCE is strong in that it makes a mutable instance of what is normally an immutable object. But the exploit is weak; it changes the object and prints it. Certainly you can do something stronger with a nefariously-mutable object? Something related to concurrency, for instance? Such as pass it to an authentication mechanism, and then change it under the mechanism's nose?
    • I'm confused about the 2nd NCCE. Are you adding this security check to your malicious BigInteger subclass, or (theoretically) to the java.math.BigInteger class? The last statement is confusing; it looks to me that if java.math.BigInteger had this security check, then untrusted code could define the malicious BigInteger class, but could not construct an object of that class, lest it get caught by the security manager. (It probably could deserialize a malicious BigInteger object)
    1. You appear to have capitulated rather quickly.  The line you quoted above was in the original rule;  Fred did not add it (at least recently).

      • I agree this is a very broad reaching rule and would cause numerous existing classes to be considered nonconforming.  I'm not sure that in itself is sufficient to demote to a recommendation.  If something is wrong, it is wrong.  One needs to have principles.
      • I agree that nefariously-mutable objects are subject to TOCTOU attacks to circumvent authentication or input validation.

      Going to look at it in more detail again.  This was easier when I had edit privileges.

      1. To further my previous point, here is a quote from the current version:

        To mitigate these risks, by default classes should be declared final unless there is a definite need for the class to be extensible. In that case, developers must carefully design the class with extensibility in mind. As a specific instance of this rule, classes that are designed to be treated as immutable either must be declared final or must have all of their member methods and fields declared final or private.

        Note first that it still says "rule".  But, I think the rule is that the class must a) be declared final or b) carefully designed for extensibility.   Presumably classes in the JDK that don't comply with (a) comply with (b).  Under these conditions, shouldn't this remain as a rule?

        1. Capitulate??? Are we at war??? :)

          Fred and I demoted this rule to a rec because:

          • Most classes, including those in the Java core libraries do not comply with it (with the notable exception of java.lang.String. A rule that is not accepted by the community weakens the CERT Java coding standard.
          • You can write secure code that violates this rec by simply treating objects from untrusted sources as potentially malicious subclasses, as directed by OBJ06-J. Defensively copy mutable inputs and mutable internal components. Complying with that rule protects you from the consequences of violating this one.

          Bottom line, there is a fundamental design tradeoff between making classes extendible vs final. Declaring BigInteger final would have prevented malicious subclasses as this rec points out. But it would also eliminate the possibility of future-extending the class to do other useful things...a possibility the Java developers did not want to rule out. The java.lang.String class is illustrative here. It was declared final partially to prevent malicious subclasses, but primarily so that the devs could optimize strings heavily, by letting them share memory. They provided the StringBuffer class (and later StringBuilder) for those who want to extend a string class. If the library were being built today, perhaps the best solution would be to provide both final and non-final versions of many classes, including BigInteger.

          So while this rec is a good idea, it doesn't rise to the level of 'rule', and enforcing it on either the Java library or developer code is expensive and not likely to be heeded, except in code specifically designed to interact with untrusted code (eg servlet managers)

  7. For the following quote:

    The security manager denies access when it detects that a subclass without the requisite permissions is attempting to instantiate the superclass [SCG 2009].

    Which Secure Coding Guideline are you referencing?  Can you reference it in the Bibliography?