Page History

final class HandleRequest implements Runnable {
  public void run() {
    // Do something
  }
}

public final class NetworkHandler implements Runnable {
  private static ThreadGroup tg = new ThreadGroup("Chief");

  @Override public void run() {
    new Thread(tg, new HandleRequest(), "thread1").start();
    new Thread(tg, new HandleRequest(), "thread2").start();
    new Thread(tg, new HandleRequest(), "thread3").start();
  }

  public static void printActiveCount(int point) {
    System.out.println("Active Threads in Thread Group " + tg.getName() +
        " at point(" + point + "):" + " " + tg.activeCount());
  }

  public static void printEnumeratedThreads(Thread[] ta, int len) {
    System.out.println("Enumerating all threads...");
    for (int i = 0; i < len; i++) {
      System.out.println("Thread " + i + " = " + ta[i].getName());
    }
  }

  public static void main(String[] args) throws InterruptedException {
    // Start thread controller
    Thread thread = new Thread(tg, new NetworkHandler(), "controller");
    thread.start();

    // Gets the active count (insecure)
    Thread[] ta = new Thread[tg.activeCount()];

    printActiveCount(1); // P1
    // Delay to demonstrate TOCTOU condition (race window)
    Thread.sleep(1000);
    // P2: the thread count changes as new threads are initiated
    printActiveCount(2);  
    // Incorrectly uses the (now stale) thread count obtained at P1
    int n = tg.enumerate(ta);  
    // Silently ignores newly initiated threads 
    printEnumeratedThreads(ta, n); 
                                   // (between P1 and P2)

    // This code destroys the thread group if it does 
    // not have any live threads
    for (Thread thr : ta}}. A thread is assign to a thread group upon the thread's creation. If the group name is not specified explicitly, the default group called {{main}} is assigned by the Java Virtual Machine (JVM). The convenience methods of the {{ThreadGroup}} class can be used to operate on all threads in its group at once. For instance, the {{ThreadGroup.interrupt()}} method will invoke {{interrupt()}} on all threads in the thread group.  Another use for thread groups is to reinforce layered security by confining threads into groups so that they do not interfere with each other. \[[JavaThreads 04|AA. Java References#JavaThreads 04]\]

ThreadGroups are still used by Java to organize threads into a hierarchy; consequently they must be used when managing the thread hierarchy. However, many of the methods of {{ThreadGroup}} are deprecated, including {{allowThreadSuspension, resume, stop, suspend}}. Furthermore, many non-deprecated methods of {{ThreadGroup}} are seldom used because they offer little desirable functionality. Ironically, a few {{ThreadGroup}} methods are not even thread-safe. \[[Bloch 01|AA. Java References#Bloch 01]\]

Some of the insecure, yet non-deprecated methods are:

*{{ThreadGroup.activeCount()}}:* A programmer may sometimes wish to enumerate all the threads in a group as a precursor to other operations. This is accomplished by using the {{activeCount()}} method that "Returns an estimate of the number of active threads in this thread group. " \[[API 06|AA. Java References#API 06]\]. Note that there is no conclusive information on whether it returns the exact count or not; the definition of _active_ also has a different connotation here. A thread that is constructed and not started is still counted by the {{activeCount()}} method as _active_. 

*{{ThreadGroup.enumerate()}}:* According to the Java API \[[API 06|AA. Java References#API 06]\], class {{ThreadGroup}} documentation:

{quote}
\[The {{enumerate()}} method\] Copies into the specified array every active thread in this thread group and its subgroups. An application should use the {{activeCount}} method to get an estimate of how big the array should be. If the array is too short to hold all the threads, the extra threads are silently ignored. 
{quote}

Threads are removed from the thread array either when they are stopped or when their {{run()}} method has finished executing. As a result, if a thread is not started, it continues to reside in the array despite the loss of the original reference. \[[JavaThreads 99|AA. Java References#JavaThreads 99]\]

Using the {{ThreadGroup}} APIs to facilitate thread shutdown also has pitfalls. Because the {{stop()}} method is deprecated, alternative ways are required to stop threads. "One way is for the thread initiating the termination to join the other threads and so know when those threads have terminated. However, an application may have to maintain its own list of the threads it creates because simply inspecting the {{ThreadGroup}} may return library threads that do not terminate and for which join will not return." \[[JPL 06|AA. Java References#JPL 06]\].

Thread pools, as exemplified by classes in {{java.util.concurrent}} such as {{ThreadPoolExecutor}}, provide a better API for managing groups of threads, with more secure facilities for handling shutdown and thread exceptions. If a logical grouping of threads is desired, use thread pools \[[Bloch 08|AA. Java References#Bloch 08]\]. 


h2. Noncompliant Code Example

This noncompliant code example shows a {{NetworkHandler}} class that maintains a _controller_ thread {{controller}}. This thread is responsible for delegating every network request to be handled to other threads.  For the sake of brevity, it is assumed that the thread {{controller}} services these requests by starting three threads in succession. All threads are defined to belong to the same group, {{Chief}}. 

{code:bgColor=#FFcccc}
public final class NetworkHandler implements Runnable {
  private static ThreadGroup tg = new ThreadGroup("Chief");

  public void run() {
    new Thread(tg, new HandleRequest(), "thread1").start(); // Start thread 1
    new Thread(tg, new HandleRequest(), "thread2").start(); // Start thread 2	
    new Thread(tg, new HandleRequest(), "thread3").start(); // Start thread 3
  }
	
  public static void printActiveCount(int point) {
    System.out.println("Active Threads in Thread Group " + tg.getName() + 
      " at point(" + point + "):" + " " + tg.activeCount());
  }

  public static void printEnumeratedThreads(Thread[] ta, int len) {
    System.out.println("Enumerating all threads...");
    for(int i = 0; i < len; i++) {
      Systemthr.out.println("Thread " + i + " = " + ta[i].getNameinterrupt();
      while(thr.isAlive());
    }
    tg.destroy();
  }

  public static void main(String[] args) throws InterruptedException {
    Thread thread = new Thread(tg, new NetworkHandler(), "controller"); // start thread t
    thread.start();
	     
    Thread[] ta = new Thread[tg.activeCount()]; // Gets the active count (insecure) 		

    printActiveCount(1);           // At point 1 
    Thread.sleep(1000);            // Delay to demonstrate TOCTOU condition (race window)   
    printActiveCount(2);           // At point 2, the thread count changes as new threads are initiated
    int n = tg.enumerate(ta);      // Incorrectly uses the (now stale) thread count obtained at point 1	
    printEnumeratedThreads(ta, n); // Silently ignores printing newly initiated threads
                                   // (between point 1 and point 2)   

    // This code destroys the thread group if it does not have any alive threads
    for (Thread thr : ta) { 
      thr.interrupt();
      while(thr.isAlive());
    }
    tg.destroy}

public final class NetworkHandler {
  private final ExecutorService executor;

  NetworkHandler(int poolSize) {
    this.executor = Executors.newFixedThreadPool(poolSize);
  }

  public void startThreads() {
    for (int i = 0; i < 3; i++) {
      executor.execute(new HandleRequest());
    }
  }

  public void shutdownPool() {
    executor.shutdown();
  }

  public static void main(String[] args)  {
    NetworkHandler nh = new NetworkHandler(3);
    nh.startThreads();
    nh.shutdownPool();
  }
}


final class HandleRequest implements Runnable {
  public void run() {
    System.out.println("Active Threads in Thread Group " +   
      Thread.currentThread().getThreadGroup().getName()  + 
      " (Handler thread invoked this): " + " " + Thread.activeCount());	

    try {
      Thread.sleep(2000); // Corresponding to time taken to perform some task
    } catch (InterruptedException e) {
      Thread.currentThread().interrupt(); // Reset interrupted status
    }
  }
}
{code}

There is a Time of Check-Time of Use (TOCTOU) vulnerability in this implementation because obtaining the count and enumerating the list do not constitute an atomic operation. If new requests come in during the time interval between calling {{activeCount()}} and {{enumerate()}} in the {{main()}} method, the total number of threads in the group will most likely increase but the enumerated list {{ta}} will contain only the initial number, that is two thread references ({{main}} and {{controller}}). Consequently, the program will operate on stale data instead of taking into account the newly started threads in the thread group {{Chief}}.

Different runs of the program produce different values of the number of threads in the thread group {{Chief}} because of the race conditions and demonstrate how a few incoming requests can get completely ignored. Any subsequent use of the {{ta}} array is insecure. For example, using the {{destroy()}} method to destroy the thread group and its sub-groups does not work as expected. The precondition to calling {{destroy()}} is that the thread group is empty with no executing threads and the code tries to ensure this. However, when the {{destroy()}} method is called, the thread group is not empty which raises a {{java.lang.IllegalThreadStateException}}. 

{mc} The other surprise is that after enumerating array ta, Chief also consists of a thread called "main" {mc}


h2. Compliant Solution

This compliant solution uses a fixed thread pool to group its three tasks, instead of a {{ThreadGroup}}. It also uses a {{java.lang.util.concurrent.ExecutorService}} to manage the pool.  The thread pool provided does not provide an API to find the number of actively executing threads. However, the logical grouping can help control the behavior of the group as a whole. For instance, all threads belonging to a particular thread pool can be terminated at will.

{code:bgColor=#ccccff}
public final class NetworkHandler {
  private final ExecutorService executor;
  private final int poolSize;

  NetworkHandler(int poolSize) {
    this.poolSize = poolSize;
    executor = Executors.newFixedThreadPool(poolSize);
  }
      
  public void startThreads() {
    executor.execute(new Runnable() {
      public void run() {
        for (int i = 0; i < poolSize; i++) {
          executor.submit(new HandleRequest());
        }
      }
    });
  }
  
  public void shutdownPool() {
    executor.shutdown(); // Orderly shutdown
  }

  public static void main(String[] args)  {
    NetworkHandler nh = new NetworkHandler(3);
    nh.startThreads();
    nh.shutdownPool(); // Shutdown the thread pool
  }
}


final class HandleRequest implements Runnable {
  public void run() {
    // Do something
  }
}
{code}

Before Java 5.0, the {{ThreadGroup}} class had to be extended because there was no other direct way to catch an uncaught exception in a separate thread.  If the application had an installed exception handler {{UncaughtExceptionHandler}}, the only way to control it was to subclass {{ThreadGroup}}. In recent versions, the {{UncaughtExceptionHandler}} was maintained on a per-thread basis using an interface enclosed by the {{Thread}} class, leaving little to no functionality for the {{ThreadGroup}} class. \[[Goetz 06|AA. Java References#Goetz 06]\], \[[Bloch 08|AA. Java References#Bloch 08]\]

Refer to [CON37-J. Ensure that tasks executing in a thread pool do not fail silently] for more information on using uncaught exception handlers in thread pools.


h2. Risk Assessment

Using the {{ThreadGroup}} APIs may result in race conditions, memory leaks and inconsistent object state.

|| Rule || Severity || Likelihood || Remediation Cost || Priority || Level ||
| CON17- J | low | probable | low | {color:#cc9900}{*}P6{*}{color} | {color:#cc9900}{*}L2{*}{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+CON01-J].

h2. References

\[[API 06|AA. Java References#API 06]\] Methods {{activeCount}} and {{enumerate}}, Classes ThreadGroup and Thread
\[[JavaThreads 04|AA. Java References#JavaThreads 04]\] 13.1 ThreadGroups
\[[Bloch 01|AA. Java References#Bloch 01]\] Item 53: Avoid thread groups
\[[Bloch 08|AA. Java References#Bloch 08]\] Item 73: Avoid thread groups
\[[Goetz 06|AA. Java References#Goetz 06]\] 7.3.1. Uncaught Exception Handlers
\[[JPL 06|AA. Java References#JPL 06]\] 23.3.3. Shutdown Strategies
\[[SDN 06|AA. Java References#SDN 06]\] Bug ID: 4089701 and 4229558

----
[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!|CON16-J. Do not expect sleep(), yield() and getState() methods to have any synchronization semantics]&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!|11. Concurrency (CON)]&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!|CON18-J. Always invoke wait() and await() methods inside a loop]