Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

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
bgColor#FFCCCC
import java.util.HashSet;
 
public class ShortSet {
  

Wiki Markup
A Boxing conversion converts the values of a primitive type to the corresponding values of the reference type, for instance, from {{int}} to the type {{Integer}} \[[JLS 5.1.7 Boxing Conversion|http://java.sun.com/docs/books/jls/third_edition/html/conversions.html#5.1.7]\].  It can be convenient in many cases where an object parameter is desired, such as with collection classes like {{Map}} and {{List}}. It also helps avoid clutter in code.

Autoboxing can automatically wrap the primitive type to the corresponding wrapper object. But one must always be careful about this process, especially while performing comparisons. Section 5.1.7 of JLS 3rd Edition explains this point 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."

Noncompliant Code Example

This code uses == to compare two integer objects. From EXP03-J we know that for == to return true for two object references, they must point to the same underlying object. We thus deduce that the results of using the == operator here will be misleading.

Code Block
bgColor#FFCCCC

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);
 
 }
}

These comparisons generate the sequence: true, false, false and true. The cache in the Integer class can only make the number from -127 to 128 refer to the same object, which clearly explains the output of the above code. To avoid making such mistakes, when you need to compare wrapper classes, use equals instead of == (see EXP03-J for details).

Compliant Solution

Using object1.equals(object2) only compares their values. Now, the results will be true, as expected.

Code Block
bgColor#CCCCFF

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));

 }
}

Noncompliant Code Example

Sometimes a dynamic array of integers is desired. 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, ArrayList<Integer> can be used to achieve this goal.

Code Block
bgColor#FFCCCC

import java.util.ArrayList;
public class TestWrapper1 {
 public static void main(String[] args) {
  //Create an array list of integers, where each element
  //is greater 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, where 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) == list2.get(i)) counter++;
     //output the total equal number
     System.out.println(counter);
 }
}

In JDK 1.6.0_10, the output of this code snippet is 0. In this code, we want to count the same numbers of array list1 and array list2. Undoubtedly, the result is not what we expect. Integer can only cache from -127 to 128, so when an int number is beyond this range, it will be autoboxed into different objects, and == will return false. But if we can set more caches inside Integer (cache all the integer values -32K-32K, which means that all the int values could be autoboxed to the same Integer object), then the result may be different.

Compliant Solution

< 10; i++) {
	  s.add((short)i); // Cast required so that the code compiles
	  s.remove(i); // Tries to remove an Integer
	}
	System.out.println(s.size());
  }
}

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 typeIn JDK 1.6.0_10, the output of this code is 10. Notice that equals has been used for comparisons in this case.

Code Block
bgColor#CCCCFF
import java.util.HashSet;
 
public class TestWrapper1ShortSet {
  public static void main(String[] args) {
  //Create an array list of integers, where 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, where each element
  //is the same as 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++;
 
     System.out.println(counter);
 }
}

Noncompliant Code Example

Wiki Markup
This noncompliant code snippet \[[Techtalk 07|AA. Java References#Techtalk 07]\] prints {{100}} as the size of the {{HashSet}} while it is expected to print {{1}}. The combination of a {{short}} and an  {{integer}} value in the operation {{i-1}} leads to autoboxing into an {{Integer}} object. The {{HashSet}} contains only {{short}} values whereas (distinctly typed) {{Integer}} objects are being removed successively. The remove operation is thus equivalent to a _No Operation_ (NOP). The compiler enforces type checking so that only {{short}} values are inserted, however, a programmer is free to remove an object of any type without triggering any exceptions since {{Collections<E>.remove}} takes an Object parameter and not {{E}}. Such behavior can result in unintended object retention or memory leaks.

Code Block
bgColor#FFCCCC

public class ShortSet {
  public static void main(String[] args) {
    HashSet<Short> s = new HashSet<Short>();
      for(short i=0; i<100;i++) {
        s.add(i);
        s.remove(i - i);
      }
    < 10; i++) {
	  s.add((short)i);
	  // Remove a Short
	  if (s.remove((short)i) == false) {
	    System.err.println("Error removing " + i);
	  }
	}
	System.out.println(s.size());
  }
}

Compliant Solution

Avoid mixing different types together with an integer type. If inevitable, cast the autoboxed Integer object to a Short to remedy the issue described in the noncompliant code.

Code Block
bgColor#CCCCFF

public class ShortSet {
  public static void main(String[] args) {
    HashSet<Short> s = new HashSet<Short>();
      for(short i=0; i<100;i++) {
        s.add(i);
        s.remove((short)(i-1));  //cast to short 
      }
    System.out.println(s.size());
  }
}

Risk Assessment

Using array lists with primitive types causes a potential security risk.

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

Recommendation

Severity

Likelihood

Remediation Cost

Priority

Level

EXP05

EXP04-J

low

Low

probable

Probable

high

Low

P2

P6

L3

L2

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

References

Wiki Markup
\[[Core Java 04|AA. Java References#Core Java 04]\] Chapter 5 
\[[JLS 05|AA. Java References#JLS 05]\] Section 5.1.7
\[[Techtalk 07|AA. Java References#Techtalk 07]\] "The Joy of Sets"

Automated Detection

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.

ToolVersionCheckerDescription
PVS-Studio

Include Page
PVS-Studio_V
PVS-Studio_V

V6066
SonarQube
Include Page
SonarQube_V
SonarQube_V
S2175Inappropriate "Collection" calls should not be made


Bibliography


...

Image Added Image Added Image AddedEXP04-J. Be wary of invisible implicit casts      02. Expressions (EXP)      03. Scope (SCP)