CON32-J. Synchronize access to static fields that may be modified by untrusted code

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.

Documented design intent is irrelevant when dealing with untrusted code because an attacker can always chose to ignore the documentation.

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++;
  }
}

This class definition does not violate CON01-J. Ensure that compound operations on shared variables are atomic which only applies to classes that promise thread-safety. However, this class has a mutable static field counter that is modified by the publicly accessible incrementCounter() method. Consequently, this class cannot be securely used by trusted client code if untrusted code can purposely fail to externally synchronize access to the field.

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 solution also complies with 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 internally synchronize access to static fields that may be modified by untrusted code will result in incorrectly synchronized code if the author of the untrusted code chooses to 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]]
[[Bloch 08]] Item 67: "Avoid excessive synchronization"

Issue Tracking

CON31-J. Avoid client-side locking when using classes that do not commit to their locking strategy      11. Concurrency (CON)      CON33-J. Document thread-safety and use annotations where applicable