sid-ramakrishnan / ScoreOverFlow

0 stars 0 forks source link

Fail to protect class fields #3

Open Debug1995 opened 5 years ago

Debug1995 commented 5 years ago

1.1 Final static field is a mutable array: Ex: ReservationDaoTestUtils.java[line 25]:

public static final int[] RESERVATION_IDS = new int[] { 1, 2 };

Spotbugs Explanation: A final static field references an array and can be accessed by malicious code or by accident from another package. This code can freely modify the contents of the array.

Our Analysis: The array, RESERVATION_IDS, is intended to by an array of constants, meaning that its contents are not to be modified. However, this code declares it as a constant array. By adding the modifier “static final” to the array, RESERVATION_IDS cannot be changed to reference another array, but the contents of the same array are free to edit. Alternatively, the values 1 and 2 should be declared as standalone constant fields.

1.2 Field should be package protected Ex: SecurityService.java

public static Subject currentUser;

Spotbugs Explanation: A mutable static field could be changed by malicious code or by accident. The field could be made package protected to avoid this vulnerability.

Our Analysis: This field, currentUser, a public static field. This means that is someone changes the value of this field, it would affect all the instances within the program, even those who do not anticipate the change. A better way is to add the protected modifier to the field and set this value only through a set public method outside of the package.

1.3 Field isn't final but should be Ex: DaoFactory.java[line 8]:

public class DaoFactory { public static EntityManagerFactory factory = Persistence.createEntityManagerFactory("ScoreOverFlowPU"); }

Spotbugs Explanation: This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.

Our Analysis: The field factory is declared as static rather than final static. In this case, this field could be changed by malicious code.