CON32-J. Protect accessible mutable static fields from untrusted code

Class methods that can be invoked from untrusted code to modify a static field must synchronize access to that field. This is necessary because there is no guarantee that untrusted clients will externally synchronize when accessing the field. Because a static field is shared by all clients, untrusted clients may violate the contract by failing to provide suitable locking.

According to Joshua Block [[Bloch 08]]:

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.

Protecting the fields of a class is unnecessary when it is designed for single-threaded use. Such classes are required to document their lack of thread-safety. For example, the Java Platform, Standard Edition 6 API Specification for the java.lang.StringBuilder class states [[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 to StringBuffer as it will be faster under most implementations.

Multithreaded clients must synchronize accesses to classes whose documentation explicitly specifies the class is not thread-safe or fails to provide any assurances that the class is safe-thread.

Noncompliant Code Example

This noncompliant code example does not synchronize access to the static field counter.

/** This class is not thread-safe! */
public final class CountHits {
  private static int counter;
  
  public void incrementCounter() {
    counter++;
  }
}

The class relies on clients to externally synchronize the object and this class documents its lack of thread-safety. This code is noncompliant, not because it violates [CON01-J. Ensure that compound operations on shared variables are atomic], and not because it documents its lack of thread-safety, but rather because it has an accessible mutable static field counter. Untrusted code is free to modify CountHits.counter with no respect for this class's documented lack of thread-safety.

Compliant Solution

This compliant solution uses a static private final lock to protect the counter field and consequently, does not depend on any external synchronization. This is recommended by [CON04-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 void incrementCounter() {
    synchronized (lock) {
      counter++;
    }
  }
}

Risk Assessment

Failing to protect classes containing accessible static members can result in unexpected results when a client fails to obey the classes' synchronization policy.

Automated Detection

TODO

Related Vulnerabilities

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

References

[[API 06]]
[[Bloch 08]] Item 67: "Avoid excessive synchronization"

Issue Tracking

[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!]      null