CON04-J. Synchronize using a private final lock object

The synchronized keyword is used to acquire a mutual-exclusion lock so that no other thread can acquire the lock while it is being held by the executing thread. There are two ways to synchronize access to shared mutable variables, method synchronization and block synchronization.

A method declared as synchronized always uses the object's monitor (intrinsic lock) as does code that synchronizes on the this reference using a synchronized block. Poorly synchronized code is prone to contention and deadlock. An attacker can manipulate the system to trigger these conditions and cause a Denial of Service (DoS) by obtaining and indefinitely holding the intrinsic lock of an accessible class.

This vulnerability can be prevented by using a java.lang.Object declared within the class as private and final. The object must be explicitly used for locking purposes in synchronized blocks within the class's methods. This intrinsic lock is associated with the instance of the private object and not the class. Consequently, there is no lock contention between this class's methods and methods of a hostile class. Joshua Bloch refers to this as the the "private lock object" idiom. [[Bloch 01]].

Static state has the same potential problem. If a static method is declared synchronized, the intrinsic lock of the class object is acquired before any statements in its body are executed, and released when the method completes. Any untrusted code that can access an object of the class, or a subclass, can use the getClass() method to gain access to the class object. The private final lock object idiom can be used to protect static data by declaring the lock as private, static and final. Reducing the accessibility of the class to package-private adds further protection against untrusted callers.

This idiom is also suitable for classes designed for inheritance. If a superclass thread requests a lock on the object's monitor, a subclass thread can interfere with its operation. For example, a subclass may use the superclass object's intrinsic lock for performing unrelated operations, causing significant lock contention. Also, excessive use of the same lock frequently results in deadlocks. This idiom separates the locking strategy of the superclass from that of the subclass. It also permits fine-grained locking because multiple lock objects can be used for seemingly unrelated operations. This increases the overall responsiveness of the application.

An object should use a private final lock object rather than its own intrinsic lock unless the class can guarantee that untrusted code cannot:

• subclass the class or its superclass (trusted code is allowed to subclass the class)
• create an object of the class, its superclass, or subclass
• access or acquire an object instance of the class, its superclass, or subclass

If a class uses a private final lock to synchronize shared data, subclasses must also use a private final lock. However, if a class uses intrinsic synchronization over the class object without documenting its locking policy, subclasses may not use intrinsic synchronization over their own class object, unless they explicitly document their locking policy. If the superclass documents its policy by stating that client-side locking is supported, the subclasses have the option of choosing between intrinsic locking over the class object and an internal private lock. Regardless of which is chosen, subclasses must document their locking policy. Refer to the guideline CON10-J. Do not override thread-safe methods with methods that are not thread-safe for related information.

If these restrictions are not met, the object's intrinsic lock is not trustworthy. If all conditions are satisfied, then the object gains no significant security from using a private final lock object, and may synchronize using its own intrinsic lock.

Noncompliant Code Example (method synchronization)

This noncompliant code example exposes instances of the someObject class to untrusted code.

public class SomeObject {
  public synchronized void changeValue() { // Locks on the object's monitor
    // ...   
  }
}

// Untrusted code
synchronized (someObject) {
  while (true) {
    Thread.sleep(Integer.MAX_VALUE); // Indefinitely delay someObject
  }
}

The untrusted code attempts to acquire a lock on the object's monitor and upon succeeding, introduces an indefinite delay which prevents the synchronized changeValue() method from acquiring the same lock. Note that the attacker intentionally violates CON20-J. Do not perform operations that may block while holding a lock in the untrusted code.

Noncompliant Code Example (public non-final lock object)

This noncompliant code example locks on a public non-final object in an attempt to use a lock other than SomeObject's intrinsic lock.

public class SomeObject {
  public Object lock = new Object();

  public void changeValue() {
    synchronized (lock) {
      // ...
    }
  }
}

However, it is possible for untrusted code to change the value of the lock object and disrupt proper synchronization.

Noncompliant Code Example (publicly-accessible non-final lock object)

This noncompliant code example synchronizes on a private but non-final field.

public class SomeObject {
  private volatile Object lock = new Object();

  public void changeValue() {
    synchronized (lock) {
      // ...
    }
  }

  public void setLock(Object lockValue) {
    lock = lockValue;
  }
}

Any thread can modify the field's value to refer to some other object in the presence of an accessor such as setLock(). This might cause two threads that intend to lock on the same object to lock on different objects, enabling them to execute the two critical sections in an unsafe manner. For example, if one thread is in its critical section and the lock is changed, a second thread will lock on the new reference instead of the old one.

A class that provides no accessible methods to change the lock is secure against untrusted manipulation, however, it is not secure against inadvertent manipulation from within the class. Consequently, this solution is not recommended for maintainability reasons.

Compliant Solution (private final lock object)

Thread-safe public classes that may interact with untrusted code must use a private final lock object. Existing classes that use intrinsic synchronization must be refactored to use block synchronization on a private final lock object. In this compliant solution, calling changeValue() obtains a lock on a private final Object instance that is inaccessible from callers outside the class's scope.

public class SomeObject {
  private final Object lock = new Object(); // private lock object

  public void changeValue() {
    synchronized (lock) { // Locks on the private Object
      // ...
    }
  }
}

A private final lock object can only be used with block synchronization. Block synchronization is preferred over method synchronization, because operations that do not require synchronization can be moved outside the synchronized region, reducing lock contention and blocking. Note that there is no need to declare lock as volatile because of the strong visibility semantics of final fields. Instead of using setter methods to change the lock, declare and use multiple private final lock objects to satisfy the granularity requirements.

Noncompliant Code Example (static)

This noncompliant code example exposes the class object of someObject to untrusted code.

public class SomeObject {
  public static synchronized void ChangeValue() { // Locks on the class object's monitor
    // ...   
  }
}

// Untrusted code
synchronized (someObject.getClass()) {
  while (true) {
    Thread.sleep(Integer.MAX_VALUE); // Indefinitely delay someObject
  }
}

The untrusted code attempts to acquire a lock on the class object's monitor and upon succeeding, introduces an indefinite delay which prevents the synchronized changeValue() method from acquiring the same lock.

A compliant solution must comply with CON32-J. Internally synchronize classes containing accessible mutable static fields. However, the attacker intentionally violates CON20-J. Do not perform operations that may block while holding a lock in the untrusted code.

Compliant Solution (static)

Thread-safe public classes that may interact with untrusted code and use intrinsic synchronization over the class object must be refactored to use a static private final lock object and block synchronization.

public class SomeObject {
  private static final Object lock = new Object(); // private lock object

  public static void ChangeValue() {
    synchronized (lock) { // Locks on the private Object
      // ...
    }
  }
}

In this compliant solution ChangeValue() obtains a lock on a static private Object that is inaccessible from the caller.

Exceptions

EX1: A class may violate this guideline, if all the following conditions are met:

• it sufficiently documents that callers must not pass objects of this class to untrusted code,
• the class does not invoke methods on objects of any untrusted classes that violate this guideline directly or indirectly,
• the synchronization policy of the class is properly documented.

A client may use a class that violates this guideline, if all the following conditions are met:

• it does not not pass objects of this class to untrusted code
• it does not use any untrusted classes that violate this guideline directly or indirectly

EX2: If a superclass of the class documents that it supports client-side locking and synchronizes on its class object, the class should also support client-side locking in the same way and document this policy. If instead the superclass uses an internal private lock, the derived class should document its own locking policy.

EX3: A package-private class may violate this guideline because its accessibility protects against untrusted callers, however, this condition should be explicitly documented so that trusted code within the same package does not reuse or change the lock object inadvertently.

Risk Assessment

Exposing the class object to untrusted code can result in denial-of-service.

Related Vulnerabilities

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

References

[[Bloch 01]] Item 52: "Document Thread Safety"

CON03-J. Do not use background threads during class initialization      11. Concurrency (CON)      CON05-J. Do not invoke Thread.run()