Methods that can both modify a static field and be invoked from untrusted code must synchronize access to the static field. Even when client-side locking is a specified requirement of the method, untrusted clients can fail to synchronize (whether inadvertently or maliciously). Because the static field is shared by all clients, untrusted clients may violate the contract by failing to provide suitable locking.
According to Joshua Bloch [Bloch 2008]:
If a method modifies a static field, you must synchronize access to this field, even if the method is typically used only by a single thread. It is not possible for clients to perform external synchronization on such a method because there can be no guarantee that unrelated clients will do likewise.
Documented design intent is irrelevant when dealing with untrusted code because an attacker can always choose to ignore the documentation
Wiki Markup |
---|
Synchronizing a class is unnecessary when it is designed for single-threaded use. Such classes are required to document their lack of thread-safety. For instance, the documentation of class {{java.lang.StringBuilder}} states \[[API 06|AA. Java References#API 06]\] : |
This class is designed for use as a drop-in replacement for
StringBuffer
in places where the string buffer was being used by a single thread (as is generally the case). Where possible, it is recommended that this class be used in preference toStringBuffer
as it will be faster under most implementations.
Multithreaded clients of classes that are not thread-safe must externally synchronize any accesses if the documentation of the classes specify the lack of thread-safety, or fail to provide any conclusive information.
Classes that use mutable static
fields must always internally synchronize accesses to their fields. This is because there is no guarantee that all clients will synchronize externally when accessing the field. Because a static
field is shared by all clients, unrelated clients may violate the contract by not performing adequate synchronization.
Noncompliant Code Example
This noncompliant code example does not fails to synchronize access to the static counter
field counter
. :
Code Block | ||
---|---|---|
| ||
/* This class is not thread-safe */ public final class CountHits { private static int counter; public void incrementCounter() { counter++; } } |
It relies on clients to externally synchronize the object and specifies its unsafe behavior in the documentation. However, there is no guarantee that all unrelated (trusted or untrusted) clients will follow this adviceThis class definition complies with VNA02-J. Ensure that compound operations on shared variables are atomic, which applies only to classes that promise thread-safety. However, this class has a mutable static counter
field that is modified by the publicly accessible incrementCounter()
method. Consequently, this class cannot be used securely by trusted client code because untrusted code can purposely fail to externally synchronize access to the field.
Compliant Solution
This compliant solution internally synchronizes uses a static private final lock to protect the counter
field and consequently , does not depend on any external synchronization.lacks any dependence on external synchronization. This solution also complies with LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code.
Code Block | ||
---|---|---|
| ||
/* This class is thread-safe */ public final class CountHits { private static int counter; private static final Object lock = new Object(); public synchronized void incrementCounter() { synchronized (lock) { counter++; } } } |
Risk Assessment
Failing Failure to internally synchronize classes containing accessible static members can result in unexpected results when a client fails to obey the classes' internally synchronize access to static fields that can be modified by untrusted code risks incorrect synchronization because the author of the untrusted code can inadvertently or maliciously ignore the synchronization policy.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|
LCK05-J |
Low |
Probable |
Medium | P4 | L3 |
Automated Detection
TODO
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
References
Wiki Markup |
---|
\[[API 06|AA. Java References#API 06]\]
\[[Bloch 08|AA. Java References#Bloch 08]\] Item 67: "Avoid excessive synchronization" |
Tool | Version | Checker | Description | ||||||
---|---|---|---|---|---|---|---|---|---|
CodeSonar |
| JAVA.CONCURRENCY.UG.METH | Unguarded Method (Java) | ||||||
Parasoft Jtest |
| CERT.LCK05.IASF | Inspect accesses to "static" fields which may require synchronization |
Related Guidelines
Bibliography
[API 2014] | |
Item 67, "Avoid Excessive Synchronization" |
...
VOID CON06-J. Do not defer a thread that is holding a lock 11. Concurrency (CON)