Page History

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 unsafe behavior in a multithreaded environment. 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 to StringBuffer as it will be faster under most implementations.

Any multithreaded client must externally synchronize operations on an object that is not thread-safe if the documentation of the corresponding class states that it is not thread-safe.

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. :

/* 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 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 does not depend on any external synchronization.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.

/* 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.

Automated Detection

...

TODO

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

References

\[[API 06|AA. Java References#API 06]\] 
\[[Bloch 08|AA. Java References#Bloch 08]\] Item 67: "Avoid excessive synchronization"

Tool	Version	Checker	Description
CodeSonar	Include Page CodeSonar_V CodeSonar_V	JAVA.CONCURRENCY.UG.METH	Unguarded Method (Java)
Parasoft Jtest	Include Page Parasoft_V Parasoft_V	CERT.LCK05.IASF	Inspect accesses to "static" fields which may require synchronization

Related Guidelines

Bibliography

...

Image Added Image Added VOID CON06-J. Do not defer a thread that is holding a lock      11. Concurrency (CON)      Image Modified