It is common for developers to separate the program logic into different Developers often separate program logic across multiple classes or files to modularize code and to increase re-usability. Unfortunately, this often imposes maintenance hurdles such as having to reusability. When developers modify a superclass (during maintenance, for example), the developer must ensure that changes in superclasses do not indirectly affect subclass behavior.
Wiki Markup |
---|
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 a security attack. The class {{java.security.Provider}} extends {{java.util.Properties}} which in turn extends {{java.util.Hashtable}}. {{Provider}} inherits the {{put()}} and {{remove()}} methods from {{Hashtable}} and adds security manager checks to each. The {{Provider}} maps a cryptographic algorithm name (for example, RSA) to a class that provides its implementation. The security manager 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 07|AA. Java References#SCG 07]\] |
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 is required to use SubClass
as it contains various authentication mechanisms. A new method called overdraft
is added by the maintainer of the class SuperClass
and the extending class SubClass
is not aware of this change. This exposes the client application to malicious invocations. One such example is of the overdraft
method being used on the currently in-use object. All security checks are deemed useless in this case.
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.
Code Block | ||
---|---|---|
| ||
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();
// Enforce security manager check
boolean result = account.withdraw(200.0);
System.out.println("Withdrawal successful? " + result);
}
}
|
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:
Code Block | ||
---|---|---|
| ||
class Account {
// Maintains all banking-related data such as account balance
private double balance = 100;
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();
// Enforce security manager check
boolean result = account.withdraw(200.0);
if (!result) {
result = account.overdraft();
}
| ||
Code Block | ||
| ||
class SuperClass { // The bank SuperClass 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 ia 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("YouWithdrawal dosuccessful? not" have permission/input validation failed!"+ result); 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
This compliant solution overrides the overdraft()
method and throws an exception to prevent misuse of the overdraft feature.
}
}
|
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:
Code Block |
---|
public class MaliciousClient {
public static void main(String[] args) {
Account account = new BankAccount();
// No security check performed
boolean result = account.overdraft();
System.out.println("Withdrawal successful? " + result);
}
}
|
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.
Code Block | ||
---|---|---|
| ||
class BankAccount extends Account {
// ...
@Override boolean overdraft() { // Override
| ||
Code Block | ||
| ||
protected void overdraft() { // this method ia added at a later date throw new IllegalAccessException(); } } |
Noncompliant Code Example
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 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 that indicates whether or not the Calendar
represents a time after the time that represented by the specified Object
parameter. The programmer wishes to extend this functionality and return so that the after()
method returns true
even when the two objects are equal. Note that represent the same date. The programmer also overrides the method compareTo()
is also overridden in this example, to provide a "comparisons by day" option to clients . For (for example, comparing today's day date with the first day of the week (, which differs from country to country) among countries, 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 internally uses class Object's
compareTo()
method. Consequently, 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.
Code Block | ||
---|---|---|
| ||
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(),
| ||
Code Block | ||
| ||
class CalendarSubclass extends Calendar { @Override public boolean after(Object when) { if(when instanceof Calendar && super.compareTo((Calendar)when) == 0) // correctly calls CalendaranotherCalendar.compareTogetFirstDayOfWeek()); } private int return true;compareDays(int currentFirstDayOfWeek, return super.after(when); // calls CalendarSubclass.compareTo() due to polymorphism } @Override public int compareTo(Calendar anotherCalendar) { // This methodint is erroneously invoked by Calendar.after()anotherFirstDayOfWeek) { return compareTo(anotherCalendar.getFirstDayOfWeek(),anotherCalendar); } (currentFirstDayOfWeek > anotherFirstDayOfWeek) ? 1 private int compareTo(int firstDayOfWeek, Calendar c) { : int(currentFirstDayOfWeek thisTime == c.get(Calendar.DAY_OF_WEEK)anotherFirstDayOfWeek) ? 0 : -1; 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 Date of otherlast abstractSunday 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(before now) cs1.set(Calendar.DAY_OF_WEEK, Calendar.SUNDAY); // Wed Dec 31 19:00:00 EST 1969 CalendarSubclass cs2 = new CalendarSubclass(); // forwardsExpected to print true 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
. Consequently, 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 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.
System.out.println(cs1.after(cs2));
}
// Implementation of other Calendar abstract methods
}
|
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 thisCalendar
represents a time after the time represented by the specifiedObject
. This method is equivalent to
compareTo(when) > 0
if and only ifwhen
is aCalendar
instance. 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:
Code Block | ||
---|---|---|
| ||
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, 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.
Code Block | ||
---|---|---|
| ||
// 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);
}
| ||
Code Block | ||
| ||
// 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) { // CompositeCalendarcompareToCalendarImplementation.compareTo() will not bebe called now return c.compareTo(anotherCalendar.getFirstDayOfWeek(),anotherCalendar); } } class CompositeCalendar extends privateForwardingCalendar int{ compareTo(int firstDayOfWeek, Calendar cpublic CompositeCalendar(CalendarImplementation ci) { int thisTime = c.get(Calendar.DAY_OF_WEEKsuper(ci); } return@Override public boolean after(thisTimeObject > firstDayOfWeekwhen) ?{ 1 : (thisTime == firstDayOfWeek) ? 0 : -1; } public static void main(String[] args) { CalendarImplementation ci1 = new CalendarImplementation();// This will call the overridden version, i.e. // CompositeClass.compareTo(); if (when instanceof Calendar && CalendarImplementation ci2 = new CalendarImplementationsuper.compareTo(();Calendar)when) == 0) { CompositeCalendar c =// new CompositeCalendar(ci1); ci1.setTime(new Date());Return true if it is the first day of week System.out.println(c.after(ci2)); // printsreturn 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.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
OBJ01- J | medium | probable | high | P4 | L3 |
Automated Detection
TODO
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
References
Wiki Markup |
---|
\[[SCG 07|AA. Java References#SCG 07]\] Guideline 1-3 Understand how a superclass can affect subclass behavior
\[[Bloch 08|AA. Java References#Bloch 08]\] Item 16: "Favor composition over inheritance" |
// No longer compares with first day of week;
// uses default comparison with epoch
return super.after(when);
}
@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());
// Date of last Sunday (before now)
ci1.set(Calendar.DAY_OF_WEEK, Calendar.SUNDAY);
CalendarImplementation ci2 = new CalendarImplementation();
CompositeCalendar c = new CompositeCalendar(ci1);
// Expected to print true
System.out.println(c.after(ci2));
}
}
|
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
Guideline 4-6 / EXTEND-6: Understand how a superclass can affect subclass behavior |
Bibliography
[API 2014] | |
Item 16, "Favor Composition over Inheritance" | |
Design Patterns: Elements of Reusable Object-Oriented Software (p. 20) | |
"Using Prototypical Objects to Implement Shared Behavior in Object-Oriented Systems" |
...
OBJ00-J. Declare data members private 07. Object Orientation (OBJ) OBJ02-J. Avoid using finalizers