rspec/rules/S1872/java/rule.adoc

== Why is this an issue?

There is no requirement that class names be unique, only that they be unique within a package. Therefore trying to determine an object's type based on its class name is an exercise fraught with danger. One of those dangers is that a malicious user will send objects of the same name as the trusted class and thereby gain trusted access.

Instead, the ``++instanceof++`` operator or the ``++Class.isAssignableFrom()++`` method should be used to check the object's underlying type.

=== Noncompliant code example

[source,java]
----
package computer;
class Pear extends Laptop { ... }

package food;
class Pear extends Fruit { ... }

class Store {

  public boolean hasSellByDate(Object item) {
    if ("Pear".equals(item.getClass().getSimpleName())) {  // Noncompliant
      return true;  // Results in throwing away week-old computers
    }
    return false;
  }

  public boolean isList(Class<T> valueClass) {
    if (List.class.getName().equals(valueClass.getName())) {  // Noncompliant
      return true;
    }
    return false;
  }
}
----

=== Compliant solution

[source,java]
----
class Store {

  public boolean hasSellByDate(Object item) {
    if (item instanceof food.Pear) {
      return true;
    }
    return false;
  }

  public boolean isList(Class<T> valueClass) {
    if (valueClass.isAssignableFrom(List.class)) {
      return true;
    }
    return false;
  }
}
----

== Resources

* CWE - https://cwe.mitre.org/data/definitions/486[CWE-486 - Comparison of Classes by Name]
* https://wiki.sei.cmu.edu/confluence/x/eDdGBQ[CERT, OBJ09-J.] - Compare classes and not class names

ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

=== Message

Use an ["instanceof"|"isAssignableFrom()"] comparison instead.


'''
== Comments And Links
(visible only on this page)

=== on 30 Jul 2014, 21:14:24 Freddy Mallet wrote:
My feedback @Ann:

* I would have limited the scope of this rule to Java and Groovy because on my side I would not be able to say if this rule is relevant or not in {cpp}, C#, VB.Net, ...
* In the provided example in Java, I would have used the Class.getName() method and not Class.getSimpleName() which is not so widely used.
* The following extended description provided in the CWE page is for me really relevant to understand why this might be a security issue: 
____
If the decision to trust the methods and data of an object is based on the name of a class, it is possible for malicious users to send objects of the same name as trusted classes and thereby gain the trust afforded to known classes and types.

____

=== on 31 Jul 2014, 18:48:53 Ann Campbell wrote:
\[~freddy.mallet]

* I did some research at the time (& just ran through it again). All of those languages have classes and some equivalent of instanceof
* The example doesn't work with Class.getName() :-)
* I've beefed up the description.

=== on 13 Feb 2015, 17:37:16 Freddy Mallet wrote:
\[~ann.campbell.2] what should be the security category associated with this rule ?

=== on 16 Feb 2015, 12:41:40 Ann Campbell wrote:
\[~freddy.mallet] are you talking about a security-related sub-tag, or are you talking about switching the SQALE mapping to Security? Or both?

=== on 5 Apr 2015, 23:35:27 Evgeny Mandrikov wrote:
\[~ann.campbell.2] I believe that this is not applicable for {cpp} and Objective-C.


endif::env-github,rspecator-view[]