Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

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.

Noncompliant Code Example

This noncompliant code example fails to synchronize access to the static counter field:

Code Block
bgColor#FFCCCC
/*
Wiki Markup
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 instance, the documentation of class {{java.lang.StringBuilder}} states \[[API 06|AA. Java References#API 06]\] :

{quote}
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.
{quote}

Multithreaded clients of classes that are not thread-safe must protect any accesses if the documentation of the classes specify the lack of thread-safety, or fail to provide any conclusive information.

Classes that use {{static}} fields that are both publicly accessible and mutable must always protect 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 suitable locking.


h2. Noncompliant Code Example

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

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

{mc} This class is not thread-safe because it violates [CON01

This 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 uses a static private final lock to protect the counter field and 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
bgColor#ccccff
/ atomic]. {mc}
The class relies on clients to externally synchronize the object and specifies its lack of thread-safety in the documentation. This code is non-compliant, 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 lack of thread-safety.


h2. 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].

{code:bgColor=#ccccff}
/** 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++;
    }
  }
}
{code}


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

|| Rule || Severity || Likelihood || Remediation Cost || Priority || Level ||
| CON32- J | low | probable | medium | {color:green}{*}P4{*}{color} | {color:green}{*}L3{*}{color} |



h3. Automated Detection

TODO


h3. Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the [CERT website|https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+CON32-J].

h2. References

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

----
[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!|VOID CON06-J. Do not defer a thread that is holding a lock]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!|11. Concurrency (CON)]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!|CON08-J. Do not call alien methods that synchronize on the same objects as any callers in the execution chain]

Risk Assessment

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.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

LCK05-J

Low

Probable

Medium

P4

L3

Automated Detection

ToolVersionCheckerDescription
CodeSonar
Include Page
CodeSonar_V
CodeSonar_V

JAVA.CONCURRENCY.UG.METH

Unguarded Method (Java)

Parasoft Jtest
Include Page
Parasoft_V
Parasoft_V
CERT.LCK05.IASFInspect accesses to "static" fields which may require synchronization

Related Guidelines

MITRE CWE

CWE-820, Missing Synchronization

Bibliography

[API 2014]


[Bloch 2008]

Item 67, "Avoid Excessive Synchronization"


...

Image Added Image Added Image Added