OBJ01-J. Understand how a superclass can affect a subclass

It is common for developers to separate the program logic into different classes or files to encourage modularity and re-usability. Unfortunately, this often imposes maintenance hurdles such as ensuring that the superclass does not change and in turn indirectly affect subclass behavior in undesired ways.

For instance, the introduction of the entrySet method in the superclass java.util.Hashtable in JDK 1.2, left the java.security.Provider class vulnerable to malicious deletion of entries due to absence of security manager checks. (See [Guideline 1-3 Understand how a superclass can affect subclass behavior])

Noncompliant Code Example

This noncompliant example shows a class SuperClass that stores banking related information but delegates the security manager and input validation tasks to the class SubClass. The client application has to use SubClass since it contains authentication mechanisms as well. A new method called overdraft is added by the maintainer of class SuperClass and the extending class SubClass is not aware of this change. This exposes the client application to malicious invocations such as ones using the overdraft method on the currently in-use object. All security checks are deemed useless in such cases.

class SuperClass {  //Main Bank class maintains all data  during program execution
  private double balance=0;
 
  protected boolean withdraw(double amount) {
	 balance -= amount;
	 return true;	 
  }
 
  protected void overdraft() {  //this method was added at a later date
	balance += 300; //add 300 in case there is an overdraft
	System.out.println("The balance is :" + balance);
  }
}

class SubClass extends SuperClass {	//all users have to subclass this to proceed
  public boolean withdraw(double amount) {
    // inputValidation();
    // securityManagerCheck();
    // Login by checking credentials using database and then call a method in SuperClass 
    // that updates the balance field to reflect current balance, other details
    return true;
  }			
 
  public void doLogic(SuperClass sc,double amount) {
    sc.withdraw(amount);
  }
}

public class Affect {
  public static void main(String[] args) {
    SuperClass sc = new SubClass();  //override
    SubClass sub = new SubClass();  //need instance of SubClass to call methods

    if(sc.withdraw(200.0)) { //validate and enforce security manager check 
      sc = new SuperClass(); //if allowed perform the withdrawal
      sub.doLogic(sc, 200.0); //pass the instance of SuperClass to use it
    }
    else
      System.out.println("You do not have permission/input validation failed!");	
      sc.overdraft(); //newly added method, has no security manager checks. Beware!
    }
}

Compliant Solution

Always keep the following postulates in mind:

• Understand what the superclass does and watch out for mutating functionality
• Make sure that new methods that are added to the superclass are overridden appropriately if there is some division of logic
• Never modularize in absurd ways as is apparent in the noncompliant code example

Noncompliant Code Example

This noncompliant example overrides the methods after() and compareTo() of the class java.util.Calendar. The Calendar.after() method returns a boolean value depending on whether the Calendar represents a time after the time represented by the specified Object parameter. The programmer wishes to extend this functionality and return true even when the two objects are equal. Note that compareTo() is also overridden in this example, to provide a "comparisons by day" option to clients. For example, comparing today's day with the first day of week (which differs from country to country) to check whether it is a weekday.

Typically, errors manifest when assumptions are made about the implementation specific details of the superclass. Here, the two objects are compared for equality in the overriding after() method and subsequently, the superclass's after() method is explicitly called to take over. The issue is that the superclass Calendar's after() method in turn internally uses the compareTo() method. The superclass's after() method erroneously invokes the subclass's version of compareTo() due to polymorphism. Since the subclass is unaware of the superclass's implementation of after(), it does not expect any of its own overriding methods to get invoked.

class CalendarSubclass extends Calendar {
  @Override public boolean after(Object when) {
    if(when instanceof Calendar && super.compareTo((Calendar)when) == 0)
        // correctly calls Calendar.compareTo()
      return true;
    return super.after(when); // calls CalendarSubclass.compareTo() due to polymorphism
  }
	
  @Override public int compareTo(Calendar anotherCalendar) {
    // This method is erroneously invoked by Calendar.after()
    return compareTo(anotherCalendar.getFirstDayOfWeek(),anotherCalendar);
  }

  private int compareTo(int firstDayOfWeek, Calendar c) {
    int thisTime = c.get(Calendar.DAY_OF_WEEK);
    return (thisTime > firstDayOfWeek) ? 1 : (thisTime == firstDayOfWeek) ? 0 : -1;
  }

  public static void main(String[] args) {
    CalendarSubclass cs1 = new CalendarSubclass();
    CalendarSubclass cs2 = new CalendarSubclass();
    cs1.setTime(new Date());
    System.out.println(cs1.after(cs2));  // prints false
  }

/* Implementation of other abstract methods */
}

// The implementation of java.util.Calendar.after() method is shown below
public boolean after(Object when) {
  return when instanceof Calendar && compareTo((Calendar)when) > 0;
     // forwards to the subclass's implementation erroneously
}

Compliant Solution

This compliant solution recommends the use of a design technique called composition and forwarding (sometimes also referred to as delegation). A new forwarder class that contains a private member field of the Calendar type is introduced. Such a composite class constitutes composition. In this example, the field refers to CalendarImplementation, a concrete instantiable implementation of the abstract Calendar class. A wrapper class called CompositeCalendar is also introduced. It consists of the same overridden methods that constituted CalendarSubclass in the preceding noncompliant code example.

Note that each method of the class ForwardingCalendar redirects to methods of the contained class instance (CalendarImplementation), and receives back return values. This is the forwarding mechanism. This class is largely independent of the implementation of the class CalendarImplementation. Therefore, any future changes to the latter will not break CompositeCalendar which inherits from ForwardingCalendar. When CompositeCalendar's overriding after() method is invoked, it performs the necessary comparison by using the local version of the compareTo() method as required. Using super.after(when) forwards to the ForwardingCalendar which invokes the CalendarImplementation's after() method. In this case, CalendarImplementation's compareTo() method gets called instead of the overriding version in CompositeClass that was inappropriately called in the noncompliant code example, as a product of polymorphism.

// The CalendarImplementation object is a concrete implementation of the abstract Calendar class
// Class ForwardingCalendar
public class ForwardingCalendar {
  private final CalendarImplementation c;

  public ForwardingCalendar(CalendarImplementation c) {
    this.c = c;
  }

  public boolean after(Object when) {
    return c.after(when);
  }

  public int compareTo(Calendar anotherCalendar) {
    // ForwardingCalendar's compareTo() will be called
    return c.compareTo(anotherCalendar);
  }
}

//Class CompositeCalendar
class CompositeCalendar extends ForwardingCalendar {
  public CompositeCalendar(CalendarImplementation ci) {
    super(ci);  
  }
  
  @Override public boolean after(Object when) {
    if(when instanceof Calendar && super.compareTo((Calendar)when) == 0)
          // this will call the overridden version
          // i.e. CompositeClass.compareTo();
          // return true if it is the first day of week
      return true;
    return super.after(when); // does not compare with first day of week anymore;
                              // uses default comparison with epoch
  }
	
  @Override public int compareTo(Calendar anotherCalendar) {
     // CompositeCalendarcompareTo() will not be called now
     return compareTo(anotherCalendar.getFirstDayOfWeek(),anotherCalendar);
  }

  private int compareTo(int firstDayOfWeek, Calendar c) {
    int thisTime = c.get(Calendar.DAY_OF_WEEK);
    return (thisTime > firstDayOfWeek) ? 1 : (thisTime == firstDayOfWeek) ? 0 : -1;
  }

  public static void main(String[] args) {
    CalendarImplementation ci1 = new CalendarImplementation();
    CalendarImplementation ci2 = new CalendarImplementation();
    CompositeCalendar c = new CompositeCalendar(ci1);
    ci1.setTime(new Date());
    System.out.println(c.after(ci2)); // prints true 
  }
}

This technique allows a superclass to evolve without causing much distress to its extending classes.

Risk Assessment

Modifying a superclass without considering the effect on a subclass can introduce vulnerabilities. Subclasses that are unaware of superclass implementations can be subject to erratic behavior resulting in inconsistent data state and mismanaged control flow.

Automated Detection

TODO

Related Vulnerabilities

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

References

[[SCG 07]] Guideline 1-3 Understand how a superclass can affect subclass behavior
[[Bloch 08]] Item 16: "Favor composition over inheritance"

OBJ00-J. Declare data members private      06. Object Orientation (OBJ)      OBJ02-J. Avoid using finalizers