The interfaces of the Java Collections Framework [JCF 2014] use generically typed, parameterized methods, such as add(E e)
and put(K key, V value)
, to insert objects into the collection or map, but they have other methods, such as contains()
, remove()
, and get()
, that accept an argument of type Object
rather than a parameterized type. Consequently, these methods accept an object of any type. The collections framework interfaces were designed in this manner to maximize backwards compatibility, but this design can also lead to coding errors. Programmers must ensure that arguments passed to methods such as Map<K,V>
get()
, Collection<E>
contains()
, and remove()
have the same type as the parameterized type of the corresponding class instance.
Noncompliant Code Example
After adding and removing 10 elements, the HashSet
in this noncompliant code example still contains 10
elements and not the expected 0. Java's type checking requires that only values of type Short
can be inserted into s. Consequently, the programmer has added a cast to short
so that the code will compile. However, the Collections<E>.remove()
method accepts an argument of type Object
rather than of type E
, allowing a programmer to attempt to remove an object of any type. In this noncompliant code example, the programmer has neglected to also cast the variable i
before passing it to the remove()
method, which is autoboxed into an object of type Integer
rather than one of type Short
. The HashSet
contains only values of type Short
; the code attempts to remove objects of type Integer
. Consequently, the remove()
method has no effect.
Code Block | ||
---|---|---|
| ||
import java.util.HashSet;
public class ShortSet {
public |
"Ideally, boxing a given primitive value p, would always yield an identical reference. In practice, this may not be feasible using existing implementation techniques. The rules above are a pragmatic compromise. The final clause above requires that certain common values always be boxed into indistinguishable objects. The implementation may cache these, lazily or eagerly."(From section 5.1.7 of JLS 3rd Ed)
Autoboxing can automatically wrap the primitive type to the corresponding wrapper object, which can be convenient in many cases and avoid clutters in your own code. But you should always be careful about this process, especially when comparison. Consider the following code:
Noncompliant code Example
Code Block |
---|
public class TestWrapper2 {
 public static void main(String[] args) {
 Â
  Integer i1 = 100;
    Integer i2 = 100;
    Integer i3 = 1000;
    Integer i4 = 1000;
    System.out.println(i1==i2);
    System.out.println(i1!=i2);
    System.out.println(i3==i4);
    System.out.println(i3!=i4);
   Â
 }
}
|
This code uses "==" to compare 2 Integer object, from EXP03-J we can know that for "==" to return true
for two object references, they must point to the same underlying object. So we can simply draw the conclusion that the results of using "==" operator in this code will be false. However,
Output of this code
Code Block |
---|
true
false
false
true
|
Section 5.1.7 of JLS 3rd Ed can explain this problem clearly:
"If the value p being boxed is true, false, a byte, a char in the range \u0000 to \u007f, or an int or short number between -128 and 127, then let r1 and r2 be the results of any two boxing conversions of p. It is always the case that r1 == r2."
Here the cache in the Integer class can make the number from -127 to 128 refer to the same object, which clearly explains the result of above code. In case of making such mistakes, when we need to do some comparisons of these wrapper class, we should use equal instead "==" (see EXP03-J for details):
Compliant solution
Code Block |
---|
public class TestWrapper2 {
 public static void main(String[] args) {
 Â
  Integer i1 = 100;
    Integer i2 = 100;
    Integer i3 = 1000;
    Integer i4 = 1000;
    System.out.println(i1.equals(i2));
    System.out.println(i3.equals(i4));   Â
 }
}Â
|
Using object1.equals(object2) only compares their values. Now, the results will be true, as we expected.
Noncompliant Code Example
In many times, you may want to create a dynamic array of integers. Unfortunately, the type parameter inside the angle brackets cannot be a primitive type. It is not possible to form an ArrayList<int>. Thanks to the wrapper class, now you can use ArrayList<Integer> to achieve this goal.
Code Block |
---|
import java.util.ArrayList; public class TestWrapper1 {  public static void main(String[] args) {   //create an array list of integers, which each element   //is more than 127     ArrayList<Integer> list1 HashSet<Short> s = new ArrayList<Integer>HashSet<Short>();     for for(int i = 0;i<10;i++)      list1.add(i+1000);   //create another array list of integers, which each element   //is the same with the first one     ArrayList<Integer> list2 = new ArrayList<Integer>();     for(int i=0;i<10; < 10; i++) {      list2s.add((short)i+1000);                int counter = 0;     for(int i=0;i<10;i++)      if(list1.get(i) == list2.get(i)) counter++;     //output the total equal number     // Cast required so that the code compiles s.remove(i); // Tries to remove an Integer } System.out.println(counters.size());  } } |
In JDK 1.6.0_10, the output of this code is 0. In this code, we want to count the same numbers of array list1 and array list2. Undoubtedly, the result is not the same as our expectation. But if we can set more caches inside Integer (cach all the integer value(-32K-32K), which means that all the int value could be autoboxing to the same Integer object) then the result may be different!
Compliant solution
This noncompliant code example also violates EXP00-J. Do not ignore values returned by methods because the remove()
method returns a Boolean value indicating its success.
Compliant Solution
Objects removed from a collection must share the type of the collection elements. Numeric promotion and autoboxing can produce unexpected object types. This compliant solution uses an explicit cast to short
that matches the intended boxed type.
Code Block | ||
---|---|---|
| ||
import java.util.HashSet;
public class ShortSet {
public | ||
Code Block | ||
public class TestWrapper1 {  public static void main(String[] args) {   //create an array list of integers, which each element   //is more than 127     ArrayList<Integer> list1 HashSet<Short> s = new ArrayList<Integer>HashSet<Short>();     for for(int i = 0;i<10; i < 10; i++) {      list1s.add((short)i+1000);   //create another array list of integers, which each element   //is the same with the first one     ArrayList<Integer> list2 = new ArrayList<Integer>();     for(int i=0;i<10;i++)      list2.add(i+1000);                int counter = 0;     for(int i=0;i<10;i++)      if(list1.get(i).equals(list2.get(i))) counter++;     //output the total equal number     System.out.println(counter);  } } |
In JDK 1.6.0_10, the output of this code is 10.(the reason is the same as the above code example.)
Risk Assessment
We often use array list with primitive type, so it will exert a potential security risk.
// Remove a Short
if (s.remove((short)i) == false) {
System.err.println("Error removing " + i);
}
}
System.out.println(s.size());
}
}
|
Exceptions
EXP04-J-EX1: The collections framework equals()
method also takes an argument of type Object
, but it is acceptable to pass an object of a different type from that of the underlying collection/map to the equals()
method. Doing so cannot cause any confusion because the contract of the equals()
method stipulates that objects of different classes will never be equivalent (see MET08-J. Preserve the equality contract when overriding the equals() method for more information).
EXP04-J-EX2: Some Java programs, particularly legacy programs, may iterate through a collection of variously typed objects with the expectation that only those objects with the same type as the collection parameter will be operated on. An exception is allowed when there is no expectation that the operation is not a no-op.
Risk Assessment
Passing arguments to certain Java Collection Framework methods that are of a different type from that of the class instance can cause silent failures, resulting in unintended object retention, memory leaks, or incorrect program operation [Techtalk 2007].
Rule |
---|
Severity | Likelihood | Remediation Cost | Priority | Level |
---|
EXP04-J |
Low |
Probable |
Low |
P6 | L2 |
Automated Detection
TODO
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
References
Chapter 5, Core Java⢠2 Volume I - Fundamentals, Seventh Edition by Cay S. Horstmann, Gary Cornell
Publisher:Prentice Hall PTR;Pub Date:August 17, 2004.
Section 5.1.7, The Java⢠Language Specification,Third Edition by James Gosling, Bill Joy, Guy Steele, Gilad Bracha
Detection of invocations of Collection.remove()
whose operand fails to match the type of the elements of the underlying collection is straightforward. It is possible, although unlikely, that some of these invocations could be intended. The remainder are heuristically likely to be in error. Automated detection for other APIs could be possible.
Tool | Version | Checker | Description | ||||||
---|---|---|---|---|---|---|---|---|---|
PVS-Studio |
| V6066 | |||||||
SonarQube |
| S2175 | Inappropriate "Collection" calls should not be made |
Bibliography
Chapter 5, "Inheritance" | |
[JCF 2014] | The Java Collections Framework |
[JLS 2015] | |
[Seacord 2015] | |
"The Joy of Sets" |
...