Malicious Code Vulnerability - May Expose Internal Representation by Incorporating Reference to Mutable Object

Malicious code vulnerability - May expose internal representation by incorporating reference to mutable object

Date is mutable

Using that setter, someone can modify the date instance from outside unintentionally

Consider this

class MyClass {

   private Date billDate;

   public void setBillDate(Date billDate) {
      this.billDate = billDate;
   }

}

now some one can set it

MyClass m = new MyClass();

Date dateToBeSet = new Date();
m.setBillDate(dateToBeSet); //The actual dateToBeSet is set to m

dateToBeSet.setYear(...); 
//^^^^^^^^ Un-intentional modification to dateToBeSet, will also modify the m's billDate 

To avoid this, you may want to Deep-copy before setting

public void setBillDate(Date billDate) {
    this.billDate = new Date(billDate.getTime());
}

Malicious code vulnerability - May expose internal representation by returning reference to mutable object

As the error message states, you're returning internal state (chkBox is - most likely - part of the internal state of an object even though you're not showing its definition)

This can cause problems if you - for example - do

String[] box = obj.chkBox();
box[0] = null;

Since an array object, as all Java objects, is passed by reference, this will change the original array stored inside your object as well.

What you most likely want to do to fix this is a simple

return (String[])chkBox.clone();

which returns a copy of the array instead of the actual array.

Spotbugs + Java: may expose internal representation by storing an externally mutable object into QuestionPojo.map

may expose internal representation by storing an externally mutable object into QuestionPojo.map

What this is telling you is that the internal state of an instance of this class can be changed outside the class.

For example:

Map<String, String> map = new HashMap<>();
map.put("hello", "world");

QuestionPojo qp = new QuestionPojo(map);

// Both of these lines change the map stored inside qp.
map.clear();
qp.getMap().put("silly", "silly");

As to whether this external change of state is important depends on the semantics of your class - in general it is undesirable, however, because it makes it hard to reason about its behavior, because the map can be changed from afar (i.e. anywhere that has a reference to the QuestionPojo, or the map).

The solution to this is defensive copying: take a copy of the map in the constructor:

public QuestionPojo(Map<String, String> map) {
    this.map = new HashMap<>(map);
}

and return a copy of the map in the getter:

public Map<String, String> getMap() {
    return new HashMap<>(map);
}

This means that nothing outside the class has access to the map stored inside the class.

How to Fix FindBug error 'May expose internal representation by incorporating reference to mutable object' while setting Array of Objects?

You should change your member to private :

private final Hello objs[];

While declaring the member as final prevents it from being assigned after first being initialized, it doesn't prevent its individual entries from being assigned by simply writing :

Hello[] harr = {new Hello(), new Hello()};
HelloWorld hw = new HelloWorld(harr);
hw.objs[1] = new Hello(); // this would mutate the contents of your array member

Malicious code vulnerability - May expose internal representation by returning reference to mutable object - With what objects?

Sonar is not smart enough to know if an object is mutable or not. Especially if you're returning a List, it can't tell if what you're actually returning is an ArrayList, an ImmutableList or an unmodifiable list. So it doesn't emit any warning to avoid flooding you with false positives.

Arrays and Date, on the other hand, are well-known standard classes that are mutable, and for which it can safely emist this warning.

---

Related Topics