It is common for developers to separate the program logic into different classes or files to modularize code and increase re-usability. Unfortunately, this often imposes maintenance hurdles such as having to 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 java.util.Hashtable
. The Provider
maps a cryptographic algorithm name (for example, RSA) to a class that provides its implementation.
Wiki Markup |
---|
The class}}. {{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 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 {{java.security.Provider}} did not override this method to provide the necessary security manager check \[[SCG 07|AA. Java References#SCG 07]\]. This problem is commonly know as a "fragile class hierarchy" in other object-oriented languages such as C++. |
Noncompliant Code Example
This noncompliant code 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.
Code Block |
---|
|
class SuperClass { // SuperClass maintains all banking related data such as account balance
private double balance = 100;
double getBalance() {
return balance;
}
|
Code Block |
---|
|
class SuperClass { // The SuperClass class maintains all bank data during program execution
private double balance = 0;
protected boolean withdraw(double amount) {
balance -= amount;
System.out.println("Withdrawal successful. The balance is : " + balance);
return true;
}
protected void overdraft() { // thisThis method is added at a later date
balance += 300; // addAdd 300 in case there is an overdraft
System.out.println("The balanceAdded back-up amount. The balance is :" + balance);
}
}
class SubClass extends SuperClass { //all usersSubclass have to subclass this to proceedhandles authentication
public@Override booleandouble withdrawgetBalance(double amount) {
// inputValidationsecurityManagerCheck();
// securityManagerCheckreturn super.getBalance();
}
//@Override Loginboolean by checking credentials using database and then call a method in SuperClass withdraw(double amount) {
// inputValidation(), securityManagerCheck() and authenticateUser()
// that updates the balance field to reflect current balance, other details
if ((super.getBalance() - amount) >= 0) {
super.withdraw(amount);
return true;
} else {
return false;
}
}
public static 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// ...
}
}
|
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. The following class shows how a security check may be bypassed if the client and the subclass are unaware of the superclass's implementation changing.
Code Block |
---|
|
public class Client {
public static void main(String[] args) {
SuperClass sc = new SubClass(); // Override
if(sc.withdraw(200.0)) { // Validate and enforce security manager check
SystemSubClass.out.println("You do not have permission/input validation failed!"); doLogic(sc, 200.0); // Withdraw 200.0 from superclass
} else {
sc.overdraft(); // Newly added method, has no security manager checks.
}
}
}
|
Compliant Solution
This compliant solution is the same as the noncompliant code example, except that it overrides the overdraft()
method and throws an exception to prevent misuse of the overdraft feature.
...
This noncompliant code 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 represent the same date. The method 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 internally uses class Object's
compareTo()
method. Consequently, the superclass's after()
method erroneously invokes the subclass's version of compareTo()
. Because the subclass is unaware of the superclass's implementation of after()
, it does not expect any of its own overriding methods to get invoked. The guideline MET04-J. Ensure that constructors do not call overridable methods describes similar programming errors.
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); // Calls CalendarSubclass.compareTo() instead of Calendar.compareTo()
}
@Override public int compareTo(Calendar anotherCalendar) {
// This method is erroneously invoked by Calendar.after()
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 |
Code Block |
---|
|
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() instead of Calendar.compareTo()
}
@Override public int compareTo(Calendar anotherCalendar) {
//CalendarSubclass Thiscs1 method= is erroneously invoked by Calendar.after()
return compareTo(anotherCalendar.getFirstDayOfWeek(), anotherCalendar);
}
private int compareTo(int firstDayOfWeek, Calendar c) {new CalendarSubclass();
CalendarSubclass cs2 = new CalendarSubclass(); // Wed Dec 31 19:00:00 EST 1969
int thisTime = c.get(Calendar.DAY_OF_WEEK);
cs1.setTime(new Date()); return (thisTime > firstDayOfWeek) ? 1 : (thisTime == firstDayOfWeek) ? 0 : -1;
}
public// staticCurrent void main(String[] args) {day's date
CalendarSubclass cs1 = new CalendarSubclass();
System.out.println(cs1.after(cs2)); 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
Wiki Markup |
---|
This compliant solution recommends the use of a design pattern called composition and forwarding (sometimes also referred to as delegation) \[[Lieberman 86|AA. Java References#Lieberman 86]\] and \[[Gamma 95|AA. Java References#Gamma 95, p. 20]\]. 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.
Typically, errors manifest when assumptions are made about the implementation specific details of the superclass. Here, the two objects are compared 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()
. Because the subclass is unaware of the superclass's implementation of after()
, it does not expect any of its own overriding methods to get invoked. The guideline MET04-J. Ensure that constructors do not call overridable methods describes similar programming errors.
Compliant Solution
Wiki Markup |
---|
This compliant solution recommends the use of a design pattern called composition and forwarding (sometimes also referred to as delegation) \[[Lieberman 86|AA. Java References#Lieberman 86]\] and \[[Gamma 95|AA. Java References#Gamma 95, p. 20]\]. 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. |
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);
}
public int compareTo(Calendar anotherCalendar) {
// CalendarImplementation.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) {
// This will call the overridden version i.e. CompositeClass.compareTo(); |
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) {
// CalendarImplementation.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) {
// CompositeCalendar.compareTo() will not be called now
return compareDays(super.getCalendarImplementation().getFirstDayOfWeek(),
return compareTo(anotherCalendar.getFirstDayOfWeek(), anotherCalendar);
}
private int compareTocompareDays(int firstDayOfWeekcurrentFirstDayOfWeek, Calendarint canotherFirstDayOfWeek) {
intreturn thisTime(currentFirstDayOfWeek = c.get(Calendar.DAY_OF_WEEK);
> anotherFirstDayOfWeek) ?
return (thisTime > firstDayOfWeek) ? 1 : (thisTimecurrentFirstDayOfWeek == firstDayOfWeekanotherFirstDayOfWeek) ? 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
}
}
CompositeCalendar(ci1);
ci1.setTime(new Date());
System.out.println(c.after(ci2)); // prints true
}
}
|
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 ForwardingCalendar
and in turn 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.
Risk Assessment
Modifying a superclass without considering the effect on a subclass can introduce vulnerabilities. Subclasses that are unaware of the superclass implementation can be subject to erratic behavior resulting in inconsistent data state and mismanaged control flow.
...