CERT
Skip to end of metadata
Go to start of metadata

Developers often separate program logic across multiple classes or files to modularize code and to increase reusability. When developers modify a superclass (during maintenance, for example), the developer must ensure that changes in superclasses preserve all the program invariants on which the subclasses depend. Failure to maintain all relevant invariants can cause security vulnerabilities.

Noncompliant Code Example

In this code example, a class Account stores banking-related information without any inherent security. Security is delegated to the subclass BankAccount. The client application is required to use BankAccount because it contains the security mechanism.

At a later date, the maintainer of the Account class added a new method called overdraft(). However, the BankAccount class maintainer was unaware of the change. Consequently, the client application became vulnerable to malicious invocations. For example, the overdraft() method could be invoked directly on a BankAccount object, avoiding the security checks that should have been present. The following noncompliant code example shows this vulnerability:

Although this code works as expected, it adds a dangerous attack vector. Because the overdraft() method has no security check, a malicious client can invoke it without authentication:

Compliant Solution

In this compliant solution, the BankAccount class provides an overriding version of the overdraft() method that immediately fails, preventing misuse of the overdraft feature. All other aspects of the compliant solution remain unchanged.

Alternatively, when the intended design permits the new method in the parent class to be invoked directly from a subclass without overriding, install a security manager check directly in the new method.

Noncompliant Code Example (Calendar)

This noncompliant code example overrides the methods after() and compareTo() of the class java.util.Calendar. The Calendar.after() method returns a boolean value that indicates whether or not the Calendar represents a time after that represented by the specified Object parameter. The programmer wishes to extend this functionality so that the after() method returns true even when the two objects represent the same date. The programmer also overrides the method compareTo() to provide a "comparisons by day" option to clients (for example, comparing today's date with the first day of the week, which differs among countries, to check whether it is a weekday).

The java.util.Calendar class provides a compareTo() method and an after() method. The after() method is documented in the Java API Reference [API 2014] as follows:

The after() method returns whether this Calendar represents a time after the time represented by the specified Object. This method is equivalent to
compareTo(when) > 0
if and only if when is a Calendar instance. Otherwise, the method returns false.

The documentation fails to state whether after() invokes compareTo() or whether compareTo() invokes after(). In the Oracle JDK 1.6 implementation, the source code for after() is as follows:

In this case, the two objects are initially compared using the overriding CalendarSubclass.after() method, which invokes the superclass's Calendar.after() method to perform the remainder of the comparison. But the Calendar.after() method internally calls the compareTo() method, which delegates to CalendarSubclass.compareTo(). Consequently, CalendarSubclass.after() actually calls CalendarSubclass.compareTo() and returns false.

The developer of the subclass was unaware of the implementation details of Calendar.after() and incorrectly assumed that the superclass's after() method would invoke only the superclass's methods without invoking overriding methods from the subclass. MET05-J. Ensure that constructors do not call overridable methods describes similar programming errors.

Such errors generally occur because the developer made assumptions about the implementation-specific details of the superclass. Even when these assumptions are initially correct, implementation details of the superclass may change without warning.

Compliant Solution (Calendar)

This compliant solution uses a design pattern called Composition and Forwarding (sometimes also called Delegation) [Lieberman 1986], [Gamma 1995]. The compliant solution introduces a new forwarder class that contains a private member field of the Calendar type; this is composition rather than inheritance. In this example, the field refers to CalendarImplementation, a concrete instantiable implementation of the abstract Calendar class. The compliant solution also introduces a wrapper class called CompositeCalendar that provides the same overridden methods found in the CalendarSubclass from the preceding noncompliant code example.

Note that each method of the class ForwardingCalendar redirects to methods of the contained CalendarImplementation class, from which it receives return values; this is the forwarding mechanism. The ForwardingCalendar class is largely independent of the implementation of the class CalendarImplementation. Consequently, future changes to CalendarImplementation are unlikely to break ForwardingCalendar and are also unlikely to break CompositeCalendar. Invocations of the overriding after() method of CompositeCalendar perform the necessary comparison by using the CalendarImplementation.compareTo() method as required. Using super.after(when) forwards to ForwardingCalendar, which invokes the CalendarImplementation.after() method as required. As a result, java.util.Calendar.after() invokes the CalendarImplementation.compareTo() method as required, resulting in the program correctly printing true.

Risk Assessment

Modifying a superclass without considering the effect on subclasses can introduce vulnerabilities. Subclasses that are developed with an incorrect understanding of the superclass implementation can be subject to erratic behavior, resulting in inconsistent data state and mismanaged control flow. Also, if the superclass implementation changes then the subclass may need to be redesigned to take into account these changes.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

OBJ02-J

Medium

Probable

High

P4

L3

Automated Detection

Sound automated detection is not currently feasible.

Related Vulnerabilities

The introduction of the entrySet() method in the java.util.Hashtable superclass in JDK 1.2 left the java.security.Provider subclass vulnerable to a security attack. The Provider class extends java.util.Properties, which in turn extends Hashtable. The Provider class maps a cryptographic algorithm name (for example, RSA) to a class that provides its implementation.

The Provider class inherits the put() and remove() methods from Hashtable and adds security manager checks to each. These checks ensure that malicious code cannot add or remove the mappings. When entrySet() was introduced, it became possible for untrusted code to remove the mappings from the Hashtable because Provider failed to override this method to provide the necessary security manager check [SCG 2009]. This situation is commonly known as the fragile class hierarchy problem.

Related Guidelines

Secure Coding Guidelines for Java SE, Version 5.0

Guideline 4-6 / EXTEND-6: Understand how a superclass can affect subclass behavior

Bibliography

[API 2014]

Class Calendar

[Bloch 2008]

Item 16, "Favor Composition over Inheritance"

[Gamma 1995]

Design Patterns: Elements of Reusable Object-Oriented Software (p. 20)

[Lieberman 1986]

"Using Prototypical Objects to Implement Shared Behavior in Object-Oriented Systems"

 


11 Comments

  1. Having "understand" in the title makes this rule informative / non-normative.

    Preserve invariants when making changes to superclasses.

    or perhaps:

    "Preserve dependencies in subclasses when changing superclasses."

    Comments?

    1. I vote to remove this rule entirely. As written, there's no distinction between "subclasses," which are nominally the target of the rule, and arbitrary clients. This rule is not actually saying anything more than "don't violate an API when updating an implementation," which boils down to "satisfy the API you publish," which is a very general and fundamental principle of software engineering, and doesn't belong here.

  2. 1. In the "Compliant Solution (Calendar)" where is the "CalendarImplementation" class coming from?

    2. "Subclasses that are unaware of the superclass implementation" means "Subclass that are unaware of the changes to the superclass implementation"? What does it mean that subclass is unaware of superclass implementation?

    1. 1. The CalendarImplementation class is provided by the user. Its
      details don't matter for this rule, which is why we don't define it.
      I suppose we could add something like:

      2. It means the developer or maintainer of the subclass is unaware of how the superclass is built.

      I've made both these changes.

  3. In the following compliant solution:

    overdraft() is declared to return boolean in Account class but here it is changed to void overdraft(). Should we change it to

    and return true?

  4. Secure Coding Guidelines for the Java Programming Language, Version 4.0 is now available and OBJ02-J related rule is covered in "Guideline 4-6: Understand how a superclass can affect subclass behavior":

    http://www.oracle.com/technetwork/java/seccodeguide-139067.html

    We might want to update our references to the Oracle's secure coding guidelines, including this rule.

  5. The first code example is said to be non-compliant but has a compliant (blue) box. Is that a mistake or an intentional decision? The code seems to be compliant despite the heading.

    1. The first code example is blue on purpose...it is perfectly compliant. The 2nd code sample is noncompliant, and is boxed in red.

      1. OK, thanks. The words "Noncompliant" above the blue box is misleading if you do not read the preceding text. I apologize for the unnecessary notification.

        1. Agreed, I wordsmithed the example.