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
Synchronizing a class is sometimes unnecessary when a class is designed for single-threaded use. Such classes are required to document their thread-unsafe behavior. For instance, the documentation of class java.lang.StringBuilder
states:
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.
Any multithreaded client must externally synchronize operations on a thread-unsafe object if the documentation of the corresponding class states that it is thread-unsafe.
However, 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 states so in the documentation. However, there is no guarantee that all unrelated clients will do thisThis 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 does not depend on any consequently 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 external 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)