Page History

Class methods Methods that can both modify a static field and be invoked from untrusted code to modify a static field must synchronize access to that the static field. This is necessary because there is no guarantee that untrusted clients will externally synchronize when accessing the field. Because a 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.unmigrated-wiki-markup

According to Joshua Block \Bloch [[Bloch 08|AA. Java References#Bloch 08]\]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.

This guideline also applies to classes that explicitly document their lack of thread-safety. Documented design intent is irrelevant when dealing with untrusted code because an attacker can always chose choose to ignore the documentation.

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

This class definition does not violate CON01complies with VNA02-J. Ensure that compound operations on shared variables are atomic, which applies only applies to classes that promise thread-safety. However, this class has a mutable static counter field counter that is modified by the publicly accessible incrementCounter() method. Consequently, this class cannot be used securely used by trusted client code because untrusted client code that may ( 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 lacks any dependence on external synchronization. This solution also complies with CON04 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 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' Failure to 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"

Issue Tracking

...

||Completed||Priority||Locked||CreatedDate||CompletedDate||Assignee||Name||
|T|M|F|1270153006582|1270216402886|svoboda|and specifies its lack of thread-safety in the documentation. => rephrase to : and documents its lack of thread-safety.  <---- FIXED ~DS|
|T|M|F|1270152984383|1270219726077|rcs|protecting fields is a weak word. Should be changed to synchronizing access to fields.|
|T|M|F|1270152995003|1270327832435|rcs_mgr|and not because it documents its lack of thread-safety; suggest removing this|
|T|M|F|1270153016807|1270327838669|rcs_mgr|The title can also be Ensure classes containing publicly accessible mutable static fields are thread-safe|
|T|M|F|1270215734760|1270327843870|rcs_mgr|"Classes that use {{static}} fields that are both publicly accessible and mutable must always protect accesses to their fields, *regardless of their documentation.*" => A class that is internally providing synchronization would not document that it is not thread-safe. So I don't understand why you added the words in bold.|
|T|M|F|1270215935982|1270327856033|rcs_mgr|"The class relies on clients to externally synchronize the object and *this class documents* its lack of thread-safety" => "this class" occurs twice in the sentence. Should be "and documents its lack of thread safety".|
|F|M|F|1270325608947|          |dmohindr|"any assurances that the class is safe-thread."  => safe-thread should be thread-safe!|

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