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.
Code Block | ||
---|---|---|
|
Under Construction
Sensitive fields declared as public and static can be modified by untrusted code.
Noncompliant code example
Code Block |
---|
package org.apache.xpath.compiler; public class FunctionTable { public static FuncLoader m_functions; } |
An attacker can replace the function table as follows:
Code Block |
---|
FunctionTable.m_functions = <newnew_table>table; |
Replacing the function table gives the attacker access to the XPathContext
, which is used to evaluate XPath expression. Static 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 a Java runtime environment. They the Java Runtime Environment (JRE), they can be used as a covert communication channel between different application domains (e.g. by for example, through code loaded into by different class loaders).
This vulnerability was repaired in JDK v1.4.2_05.
Compliant Solution
There are severals way to tackle this problem
...
Code Block |
---|
package org.apache.xpath.compiler;
public class FunctionTable {
private static FuncLoader m_functions;
}
|
Make public static fields final.
...
This compliant solution declares the FuncLoader
static field final and treats it as a constant:
Code Block | ||
---|---|---|
| ||
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:
Code Block | ||
---|---|---|
| ||
class DataSerializer implements Serializable {
public static long serialVersionUID = 1973473122623778747L;
// ...
}
|
Compliant Solution
This compliant solution declares the serialVersionUID
field final and private:
Code Block | ||
---|---|---|
| ||
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
Tool | Version | Checker | Description | ||||||
---|---|---|---|---|---|---|---|---|---|
CodeSonar | 4.2 | FB.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 | ||||||
Eclipse | 1.0 | Implemented. The serializable class .* does not declare a static final serialVersionUID field of type long | |||||||
Coverity | 7.5 | FB.MS_SHOULD_BE_FINAL | Implemented | ||||||
Findbugs | 1.0 | MS_MUTABLE_ARRAY MS_SHOULD_BE_FINAL | Implemented | ||||||
Parasoft Jtest |
| CERT.OBJ10.SPFF CERT.OBJ10.RMO | Inspect 'static' fields which may have intended to be declared 'static final' Avoid referencing mutable fields | ||||||
SonarQube |
| S1444 | "public static" fields should be constant | ||||||
SpotBugs |
| MS_SHOULD_BE_FINAL | Implemented |
Related Guidelines
CWE-493, Critical Public Variable without Final Modifier | |
Guideline 6-10 / MUTABLE-10: Ensure public static final field values are constants |
Bibliography
[FT 2008] | "Function Table" |
Section 9.3, "Static Fields" | |
Antipattern 5, Misusing Public Static Variables | |
Antipattern 5, Misusing Public Static Variables |
...
Field isn't final but should be refactored to be so