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 When developers modify a superclass (during maintenance, for example), the developer must ensure that changes in superclasses do not indirectly affect subclass behavior.preserve all of the program invariants on which the subclasses depend. Failure to maintain all relevant invariants can cause security vulnerabilities.
The 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
. 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 security manager. 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 {{java.security.Provider}} did not override this method to provide the necessary security manager check \[[SCG 2007|AA. Bibliography#SCG 07]\]. This problem is commonly know as a "fragile class hierarchy" in other object-oriented languages such as C++. |
...
This noncompliant code example shows a class SuperClass
that stores banking-related information but and delegates the security manager and input validation tasks to the class SubClass
. The client application is required to use SubClass
as because it contains various the authentication mechanisms.
Code Block | ||
---|---|---|
| ||
class SuperClass { // SuperClass 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; } void overdraft() { // This method is added at a later date balance += 300; // Add 300 in case there is an overdraft System.out.println("Added back-up amount. The balance is :" + balance); } } class SubClass extends SuperClass { // Subclass handles authentication @Override boolean withdraw(double amount) { // inputValidation(), securityManagerCheck() and authenticateUser() return super.withdraw(amount); } public static void doLogic(SuperClass sc, double amount) { if (!sc.withdraw(amount)) { throw new IllegalStateException(); } // ... } } |
A new method called overdraft
is added by At a later date, the maintainer of the class SuperClass
and added a new method called overdraft
; the extending class SubClass
is , however, was not aware of this change. This exposes the The client application consequently became vulnerable to malicious invocations. One such For example is of , the overdraft
method being used could be invoked directly on the currently in-use SubClass
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, avoiding the security checks that should have been present. The following demonstrates this vulnerability.
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 SubClass.doLogic(sc, 200.0); // Withdraw 200.0 from superclass } else { sc.overdraft(); // Newly added method, haslacks no security manager checks } } } |
Compliant Solution
This In this compliant solution is the same as the noncompliant code example, except that it overrides , class SubClass
provides an overriding version of the overdraft()
method and that throws an exception to prevent , thus preventing misuse of the overdraft feature. All other aspects of the compliant solution remain unchanged.
Code Block | ||
---|---|---|
| ||
class SubClass extends SuperClass { // ... @Override void overdraft() { // override throw new IllegalAccessException(); } } |
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 in the overridden method if it should be allowable to use it from a subclassdirectly in the new method.
Noncompliant Code Example
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 that indicates whether 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 represent the same date. The She also overrides 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.
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) { // Note the == operator 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) { CalendarSubclass cs1 = new CalendarSubclass(); CalendarSubclass cs2 = new CalendarSubclass(); // Wed Dec 31 19:00:00 EST 1969 cs1.setTime(new Date()); // Current day's 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; // Note the > operator // forwards to the subclass's implementation erroneously } 's implementation erroneously } |
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. In this caseTypically, errors manifest when assumptions are made about the implementation specific details of the superclass. Here, the two objects are initially compared in using the overriding after()
method and ; subsequently, the superclass's after()
method is explicitly called to take over. The issue is that invoked to perform the remainder of the comparison. But 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 developer of the subclass is was unaware of the details of the superclass's implementation of after()
, it does not expect any of its own overriding methods to get invokedshe 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.
...
Wiki Markup |
---|
This compliant solution recommends the use of a design pattern called composition and forwarding (sometimes also referred to as delegation) \[[Lieberman 1986|AA. Bibliography#Lieberman 86]\] and \[[Gamma 1995|AA. Bibliography#Gamma 95, p. 20]\]. A The compliant solution introduces a new _forwarder_ class that contains a {{private}} member field of the {{Calendar}} type is introduced. Such a composite class constitutes}} type; this is _composition_. rather than inheritance In this example, the field refers to {{CalendarImplementation}}, a concrete instantiable implementation of the {{abstract}} {{Calendar}} class. A The compliant solution also introduces a wrapper class called {{CompositeCalendar}} is also introduced. It consists of that provides the same overridden methods thatfound in constitutedthe {{CalendarSubclass}} infrom the preceding noncompliant code example. |
...
Note that each method of the class ForwardingCalendar
redirects to methods of the contained class instance (CalendarImplementation
), and from which it receives back return values. This ; this is the forwarding mechanism. The ForwardingCalendar
class is largely independent of the implementation of the class CalendarImplementation
. Consequently, any future changes to the latter will not to CalendarImplementation
are unlikely to break ForwardingCalendar
and in turn thus are also unlikely to break CompositeCalendar
, which derives from it ForwardingCalendar
. When Invocations of CompositeCalendar
's overriding after()
method is invoked, it performs perform 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, Consequently, ava.util.Calendar.after()
invokes CalendarImplementation
's compareTo()
method gets called instead of rather than the overriding version in CompositeClass
that was inappropriately called in the noncompliant code example.
...
Guideline | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
OBJ07-J | medium | probable | high | P4 | L3 |
Automated Detection
TODOSound automated detection is not currently feasible.
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this guideline on the CERT website.
...