Developers often separate program logic across multiple classes or files to modularize code and to increase re-usability. When developers modify a superclass (during maintenance, for example), the developer must ensure that changes in superclasses preserve all of the program invariants on which the subclasses depend. Failure to maintain all relevant invariants can cause security vulnerabilities.
Noncompliant Code Example
This noncompliant code example relies on a class Account that stores banking-related information, with no inherent security. Security is delegated to the subclass BankAccount. The client application is required to use BankAccount because it contains the security mechanism.
private class Account { // Maintains all banking related data such as account balance
private double balance = 100;
boolean withdraw(double amount) {
if ((balance - amount) >= 0) {
balance -= amount;
System.out.println("Withdrawal successful. The balance is : " + balance);
return true;
}
return false;
}
}
public class BankAccount extends Account { // Subclass handles authentication
@Override boolean withdraw(double amount) {
if (!securityCheck()) {
throw new IllegalAccessException();
}
return super.withdraw(amount);
}
private final boolean securityCheck() {
// check that account management may proceed
}
}
public class Client {
public static void main(String[] args) {
Account account = new BankAccount();
boolean result = account.withdraw(200.0); // Enforce security manager check
System.out.println("Withdraw successful? " + result);
}
}
At a later date, the maintainer of the class Account added a new method called overdraft(). However, the BankAccount class maintainer is unaware of the change. The client application subsequently 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 code illustrates this vulnerability.
private class Account { // Maintains all banking related data such as account balance
boolean overdraft() {
balance += 300; // Add 300 in case there is an overdraft
System.out.println("Added back-up amount. The balance is :" + balance);
return true;
}
// other Account methods
}
public class BankAccount extends Account { // Subclass handles authentication
//NOTE: unchanged from previous version
//NOTE: lacks override of overdraft method
}
public class Client {
public static void main(String[] args) {
Account account = new BankAccount();
boolean result = account.withdraw(200.0); // Enforce security manager check
if (!result) {
result = account.overdraft();
}
System.out.println("Withdrawl successful? " + result);
}
}
While this code works as expected, it adds a dangerous vector of attack. Because there is no security check on the overdraft() method, a malicious client can invoke it without authentication:
public class MaliciousClient {
public static void main(String[] args) {
Account account = new BankAccount();
boolean result = account.overdraft(200.0); // No security check performed
System.out.println("Withdrawl successful? " + result);
}
}
Compliant Solution
In this compliant solution, the BankAccount class provides an overriding version of the overdraft() method that immediately fails, thereby preventing misuse of the overdraft feature. All other aspects of the compliant solution remain unchanged.
class BankAccount extends Account {
// ...
@Override void overdraft() { // override
throw new IllegalAccessException();
}
}
Alternately, 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.
Related Vulnerability: JDK 1.2 java.util.Hashtable.entrySet()
The introduction of the entrySet() method in the java.util.Hashtable superclass in JDK 1.2 left the java.security.Provider subclass class 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 did not override this method to provide the necessary security manager check [[SCG 2007]]. This problem is commonly know as a "fragile class hierarchy" in other object-oriented languages such as C++.
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 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. She also overrides the method compareTo() 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.
class CalendarSubclass extends Calendar {
@Override public boolean after(Object when) {
// correctly calls Calendar.compareTo()
if (when instanceof Calendar && super.compareTo((Calendar) when) == 0) {
return true;
}
return super.after(when);
}
@Override public int compareTo(Calendar anotherCalendar) {
return compareDays(this.getFirstDayOfWeek(), anotherCalendar.getFirstDayOfWeek());
}
private int compareDays(int currentFirstDayOfWeek, int anotherFirstDayOfWeek) {
return (currentFirstDayOfWeek > anotherFirstDayOfWeek) ?
1 : (currentFirstDayOfWeek == anotherFirstDayOfWeek) ? 0 : -1;
}
public static void main(String[] args) {
CalendarSubclass cs1 = new CalendarSubclass();
cs1.setTime(new Date());
cs1.set( Calendar.DAY_OF_WEEK, Calendar.SUNDAY); // Date of last Sunday (before now)
CalendarSubclass cs2 = new CalendarSubclass(); // Wed Dec 31 19:00:00 EST 1969
System.out.println(cs1.after(cs2)); // expected to print true
}
// Implementation of other Calendar abstract methods
}
Such errors generally occur because the developer has depended on assumptions about the implementation specific details of the superclass. Even when these assumptions are correct when originally made, the implementation details of the superclass may change in the future without warning.
The java.util.Calendar class provides a compareTo() method, and an after() method. The after() method is documented as follows: ([[API 2006]])
Returns whether this
Calendarrepresents a time after the time represented by the specifiedObject. This method is equivalent to:
compareTo(when) > 0
if and only ifwhenis aCalendarinstance. Otherwise, the method returnsfalse.
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:
public boolean after(Object when) {
return when instanceof Calendar
&& compareTo((Calendar) when) > 0;
In this case, the two objects are initially compared using the overriding CalendarSubclass.after() method. This 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 is delegated to CalendarSubclass.compareTo(). Consequently, CalendarSubclass.after() actually calls CalendarSubclass.compareTo(), and consequently returns false.
Because the developer of the subclass was unaware of the implementation details of Calendar.after(), she incorrectly assumed that the superclass's after() method would invoke only its own methods without invoking overriding methods from the subclass. The guideline MET04-J. Ensure that constructors do not call overridable methods describes similar programming errors.
Compliant Solution (Calendar)
This compliant solution uses a design pattern called composition and forwarding (sometimes also referred to as delegation) [[Lieberman 1986]] and [[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.
// 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;
}
CalendarImplementation getCalendarImplementation() {
return c;
}
public boolean after(Object when) {
return c.after(when);
}
public int compareTo(Calendar anotherCalendar) {
// CalendarImplementation.compareTo() will be called
return c.compareTo(anotherCalendar);
}
}
class CompositeCalendar extends ForwardingCalendar {
public CompositeCalendar(CalendarImplementation ci) {
super(ci);
}
@Override public boolean after(Object when) {
// This will call the overridden version i.e. CompositeClass.compareTo();
if (when instanceof Calendar && super.compareTo((Calendar)when) == 0) {
// 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) {
return compareDays(super.getCalendarImplementation().getFirstDayOfWeek(),
anotherCalendar.getFirstDayOfWeek());
}
private int compareDays(int currentFirstDayOfWeek, int anotherFirstDayOfWeek) {
return (currentFirstDayOfWeek > anotherFirstDayOfWeek) ?
1 : (currentFirstDayOfWeek == anotherFirstDayOfWeek) ? 0 : -1;
}
public static void main(String[] args) {
CalendarImplementation ci1 = new CalendarImplementation();
ci1.setTime(new Date());
ci1.set( Calendar.DAY_OF_WEEK, Calendar.SUNDAY); // Date of last Sunday (before now)
CalendarImplementation ci2 = new CalendarImplementation();
CompositeCalendar c = new CompositeCalendar(ci1);
System.out.println(c.after(ci2)); // expected to print true
}
}
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 thus are also unlikely to break CompositeCalendar. Invocations of CompositeCalendar's overriding after() method 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. Consequently, ava.util.Calendar.after() invokes the CalendarImplementation.compareTo() method as required, with the result that the program correctly prints true.
Risk Assessment
Modifying a superclass without considering the effect on subclasses can introduce vulnerabilities. Subclasses that are unaware of the superclass implementation may be subject to erratic behavior resulting in inconsistent data state and mismanaged control flow.
Guideline |
Severity |
Likelihood |
Remediation Cost |
Priority |
Level |
|---|---|---|---|---|---|
OBJ07-J |
medium |
probable |
high |
P4 |
L3 |
Automated Detection
Sound automated detection is not currently feasible.
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this guideline on the CERT website.
Bibliography
[[API 2006]] Calendar
[[Bloch 2008]] Item 16: "Favor composition over inheritance"
[[Gamma 1995]]
[[Lieberman 1986]]
[[SCG 2007]] Guideline 1-3 Understand how a superclass can affect subclass behavior
OBJ06-J. Compare classes and not class names 04. Object Orientation (OBJ) OBJ08-J. Provide mutable classes with copy functionality to allow passing instances to untrusted code safely