SEI CERT Oracle Coding Standard for Java

Space shortcuts

Page tree

Increasing the accessibility of overridden or hidden methods permits a malicious subclass to offer wider access to the restricted method than was originally intended. Consequently, programs must override methods only when necessary and must declare methods final whenever possible to prevent malicious subclassing. When methods cannot be declared final, programs must refrain from increasing the accessibility of overridden methods.

The access modifier of an overriding or hiding method must provide at least as much access as the overridden or hidden method (The Java Language Specification, §8.4.8.3, "Requirements in Overriding and Hiding" [JLS 2015]). The following table lists the allowed accesses.

Overridden/Hidden Method Modifier

Overriding/Hiding Method Modifier

public

public

protected

protected or public

default

default or protected or public

private

Cannot be overridden

Noncompliant Code Example

This noncompliant code example demonstrates how a malicious subclass Sub can both override the doLogic() method of the superclass and increase the accessibility of the overriding method. Any user of Sub can invoke the doLogic method because the base class Super defines it to be protected, consequently allowing class Sub to increase the accessibility of doLogic() by declaring its own version of the method to be public.

class Super {
  protected void doLogic() {
    System.out.println("Super invoked");
  }
}

public class Sub extends Super {
  public void doLogic() {
    System.out.println("Sub invoked");
    // Do sensitive operations
  }
}

Compliant Solution

This compliant solution declares the doLogic() method final to prevent malicious overriding:

class Super {
  protected final void doLogic() { // Declare as final
    System.out.println("Super invoked");
    // Do sensitive operations
  }
}

Exceptions

MET04-J-EX0: For classes that implement the java.lang.Cloneable interface, the accessibility of the Object.clone() method should be increased from protected to public [SCG 2009].

Risk Assessment

Subclassing allows weakening of access restrictions, which can compromise the security of a Java application.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

MET04-J

Medium

Probable

Medium

P8

L2

Automated Detection

Detecting violations of this rule is straightforward.

ToolVersionCheckerDescription
Parasoft Jtest
2023.1
CERT.MET04.OPMDo not override an instance "private" method

Related Guidelines

MITRE CWE

CWE-487, Reliance on Package-Level Scope

Secure Coding Guidelines for Java SE, Version 5.0

Guideline 4-1 / EXTEND-1: Limit the accessibility of classes, interfaces, methods, and fields

Bibliography

[JLS 2015]

§8.4.8.3, "Requirements in Overriding and Hiding"

[SCG 2009]


18 Comments

  1. David Svoboda

    Sorry, the compliant solution doesn't protect private member access from malicious subclasses. I have just compiled and successfully run this code using Sun Java 6 on Linux.

    public class Foo {
		public static void main(String[] args) {
				Foo foo = new Foo();
				Sub sub = foo.new Sub();
				sub.doLogic();
		}

		class BadScope {
				final private void doLogic() {
						System.out.println("Super invoked");
				}
		}

		public class Sub extends BadScope {
				public void doLogic() {
						System.out.println("Sub invoked");
						super.doLogic();
				}
		}
}

    It produces:

    Sub invoked
Super invoked

    demonstrating that even private final methods can be overridden.

    1. David Svoboda

      OK, I see this is addressed in SCP02, since my example uses inner classes. And I can't reproduce the access violation w/o inner classes.

      1. David Svoboda

        OK, after studying this rule some more, it seems invalid...I'm not sure what the actual goal is.

        Java allows a subclass to 'hide' a private method in a superclass, even if that method is final. So the compliant solution doesn't solve anything. Java will refuse to compile a subclass that overrides a final method that is non-private. But since a private method cannot be seen by subclasses (or invoked), there is really no harm in letting subclasses define the same method name, as the private method name is exclusive to its owning class.

        As my previous comment indicates, this is all out the window when dealing with inner classes.

        As for overriding non-private methods, Java does allow this, but this doesn't weaken the overridden method itself; its restrictions are still enforced. One could use overriding to grant public access to a protected method, eg:

        class BadScope {
  protected void doLogic() {...}
}

public class Sub extends BadScope {
  public void doLogic() {super.doLogic();}

        The scope of BadScope.doLogic() is still protected, but it can be indirectly accessed via Sub.doLogic().

        I think the only possible danger here is invoking a public method that turns out to be overridden by an evil class, eg:

        public class Good {
  public void doLogic() {...}
}

public class Evil extends Good {
  public void doLogic() {...}
}
...
Good item = new Evil();
...
if (item instanceof Good) {
  item.doLogic(); // oops, calls Evil.doLogic()
}

        One can mitigate this by declaring Good.doLogic() final (or Good itself final). Or one can explicitly call item.Good.doLogic() which avoids Evil.dologic() even if the item is Evil. Is this what the rule is getting at?

  2. Thomas Hawtin

    Technically, in the noncompliant example there is no overriding. Private methods cannot be overriden.

    A public or protected method could be overriden to do something malicious, so be careful when calling methods even when on called on 'this'.

    I think the main issue here is providing a coherent security boundary. With regards to inheritance, the best solution is to prohibit it (for sensitive classes).

  3. Dhruv Mohindra

    I am also guessing on this one since I had not added the name of this rule.
    True, one cannot "override" private methods per se.
    I concur with the possible malicious traits discussed, unless this rule meant something very different. Maybe "method" scope overiding/hiding is not outwardly too dangerous after all.

    It is also possible that this rule should've talked about "variable" scope rather than "method" scope.

    1. Dhruv Mohindra

      OBJ05-J. Limit the extensibility of non-final classes and methods to only trusted subclasses is a duplicate of this. The former can do with some text given here and a few examples on method overriding and field inheritance. I suspect OBJ33 is misclassified under OBJ.

  4. Kirk Sayre

    In the first NCCE/CE pair declaring BadScope::doLogic() private prevents Sub from accessing BadScope::doLogic() at all. If it is expected that subclasses of BadScope will need to be able to call BadScope::doLogic() (but not override doLogic()), BadScope::doLogic() may be declared as protected and final:

    public class BadScope {
    protected final void doLogic() { System.out.println("Super invoked"); }
}

    This accomplishes the goal of the recommendation without hiding BadScope::doLogic() from subclasses.

    1. Dhruv Mohindra

      Correct. That should've been protected.

  5. Dean Sutherland

    TODO Check for existing analysis tools that perform the automated check.

  6. David Svoboda

    I'm still not sure this rule should exist. Sometimes it is worthwhile to make a base protected method public in a subclass (although it is probably better to make a different subclass method delegate to the protected method). I'm just not convinced that violating this rule constitutes a vulnerability.

    Would violating this rule also violate OBJ00-J. Limit extensibility of classes and methods with invariants to trusted subclasses only?

    1. Dean Sutherland

      Violating this rule would often violate OBJ00, but not always. Imagine, for example, a pure function (e.g., no side effects, and also a given input always produces the same output which implies no dependencies on mutable state). The only "invariant" I can see for such a function would be either temporal (forex, A: don't call me before the final fields I depend on have been or can be computed, or B: I consume vast resources, so don't call me from a realtime thread, or during time-sensitive operations), or security related (only authorized callers need apply, because my result is somehow sensitive). Do those count?

      Alternatively, the overrider/extender could be trusted code that is actually permitted to override/extend. In this case, the bug would be expanding the accessibility, which could leak sensitive data, a capability, or both.

      1. David Svoboda

        I think the right question to ask is, would we nix code that violates MET17-J but does not violate OBJ00-J? Offhand I wouldn't. In that case, this rule is nonnormative, and should be voided.

      2. Dhruv Mohindra

        I seem to have commented about the similarity between this and OBJ00 in a comment ^^^. We could combine any useful info here with OBJ00 and void this.

  7. Pamela Curtis

    Not sure whether code font should be removed from public, private, and protected in the table and from protected in the noncompliant code example paragraph.

    1. Robert Seacord (Manager)

      I'm not sure how consistent this is, but I like the font treatment in the table but I would remove it from the paragraph. The paragraph also says "public" which should be treated the same way as "protected".

  8. Masaki Kubo

    Suggestion:
    change the title to "Do not increase the accessibility when overriding or hiding methods"

    1. David Svoboda

      Why? To me the suggested title is neither better nor worse than the current one.

  9. Asha

    A protected instance method in the parent class(superclass) can be made public but not private in the child class(subclass). If you try to make child class private then it will give a compile-time error. In short, you will get a compilation error if you violate §8.4.8.3, "Requirements in Overriding and Hiding" [JLS 2015]). This is one of the rules of method overriding in java.