Skip to end of metadata
Go to start of metadata

Client code can trivially access public static fields because access to such fields are not checked by a security manager. Furthermore, new values cannot be validated programmatically before they are stored in these fields.

In the presence of multiple threads, nonfinal public static fields can be modified in inconsistent ways (see TSM01-J. Do not let the this reference escape during object construction for an example).

Improper use of public static fields can also result in type-safety issues. For example, untrusted code can supply an unexpected subtype with malicious methods when the variable is defined to be of a more general type, such as java.lang.Object [Gong 2003]. As a result, classes must not contain nonfinal public static fields.

Noncompliant Code Example

This noncompliant code example is taken from JDK v1.4.2 [FT 2008]. It declares a function table containing a public static field.

package org.apache.xpath.compiler;

public class FunctionTable {
  public static FuncLoader m_functions;
}

An attacker can replace the function table as follows:

FunctionTable.m_functions = new_table;

Replacing the function table gives the attacker access to XPathContext, which is used to set the reference node for evaluating XPath expressions. Manipulating XPathContext can cause XML fields to be modified in inconsistent ways, resulting in unexpected behavior. Also, because static variables are global across the Java Runtime Environment (JRE), they can be used as a covert communication channel between different application domains (for example, through code loaded by different class loaders).

This vulnerability was repaired in JDK v1.4.2_05.

Compliant Solution

This compliant solution declares the FuncLoader static field final and treats it as a constant:

public static final FuncLoader m_functions;
// Initialize m_functions in a static initialization block

Fields declared static and final are also safe for multithreaded use (see TSM03-J. Do not publish partially initialized objects for more information). However, remember that simply changing the modifier to final might not prevent attackers from indirectly retrieving an incorrect value from the static final variable before its initialization (see DCL00-J. Prevent class initialization cycles for more information). Furthermore, individual members of the referenced object can also be changed if the object itself is mutable.

It is also permissible to use a wrapper method to retrieve the value of m_functions, allowing m_functions to be declared private (see rule OBJ01-J. Limit accessibility of fields for more information).

Noncompliant Code Example (serialVersionUID)

This noncompliant code example uses a public static nonfinal serialVersionUID field in a class designed for serialization:

class DataSerializer implements Serializable {
  public static long serialVersionUID = 1973473122623778747L;
  // ...
}

Compliant Solution

This compliant solution declares the serialVersionUID field final and private:

class DataSerializer implements Serializable {
  private static final long serialVersionUID = 1973473122623778747L;
}

The serialization mechanism uses the serialVersionUID field internally, so accessible wrapper methods are unnecessary.

Risk Assessment

Unauthorized modifications of public static variables can result in unexpected behavior and violation of class invariants. Furthermore, because static variables can be visible to code loaded by different class loaders when those class loaders are in the same delegation chain, such variables can be used as a covert communication channel between different application domains.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

OBJ10-J

Medium

Probable

Medium

P8

L2

Automated Detection

ToolVersionCheckerDescription
CodeSonar4.2FB.MALICIOUS_CODE.MS_SHOULD_BE_FINAL
FB.MALICIOUS_CODE.MS_SHOULD_BE_REFACTORED_TO_BE_FINAL
Field isn't final but should be
Field isn't final but should be refactored to be so
Eclipse1.0
Implemented. The serializable class .* does not declare a static final serialVersionUID field of type long
Coverity7.5FB.MS_SHOULD_BE_FINALImplemented
Findbugs1.0MS_MUTABLE_ARRAY MS_SHOULD_BE_FINAL Implemented
Parasoft Jtest
10.3
SECURITY.EAB.SPFF
SonarQube
6.7
S1444"public static" fields should be constant

Related Guidelines

MITRE CWE

CWE-493, Critical Public Variable without Final Modifier
CWE-500, Public Static Field Not Marked Final

Secure Coding Guidelines for Java SE, Version 5.0

Guideline 6-10 / MUTABLE-10: Ensure public static final field values are constants

Bibliography

[FT 2008]

"Function Table"
"Class Function Table"

[Gong 2003]

Section 9.3, "Static Fields"

[Nisewanger 2007]

Antipattern 5, Misusing Public Static Variables

[Sterbenz 2006]

Antipattern 5, Misusing Public Static Variables



Field isn't final but should be refactored to be so

8 Comments

  1. It's much better to solve the problem by making the field private rather than final (although it is of course good to make it final and of an immutable type). For instance in the current JDK7 FunctionTable.m_functions is an array (and non-empty arrays are mutable). Really internal classes like this should not be exposed - we'll get some form of modules one day!

    Note, the referenced bug was fixed in 1.4.2_05.

  2. This rule no longer talks about how classes loaded by different classloaders can access each others' fields. Makes the rule simpler, but it means that the Risk Assessment should provide a link to a rule that does discuss this. Since I don't know which rule discusses this, the RA currently has a broken link that should be fixed later.

    1. So I would have an exception if I use two different class loaders and try to play with each others' fields?

  3. We have two separate sources for:

    Antipattern 5, Misusing Public Static Variables

    There is probably an explanation, but it seems peculiar.

  4. Should the title of this be "OBJ10-J. Do not use public static nonfinal fields" and not "OBJ10-J. Do not use public static nonfinal variables"?  Can't variables refer to other objects, which perhaps could be public static nonfinal?

  5. This rule states that "As a result, classes must not contain nonfinal public static fields."  Should we further stipulate that this rule only applies to public or protected classes or are all classes at risk?

  6. Make it private. Problem solved. I mean honestly, if you don't want it accessible, make it private. That is the whole point of public and private. It's like saying, you should always cast int to Integer. You could, or you could let the system autobox that for you. There is a specific reason java has that. To make these complications unnecessary.

  7. As for the unexpected subtype issue, there is something in java known as instanceof. Use a switch statement for the expected types, and again, problem solved. If you need to cast, there is always try and catch if there is a ClassCastException. There are things in java that handle these issues.