Page History

 
{quote}

The use of {{ThreadLocal}} objects is insecure in classes whose objects are required to be executed by several threads, in a thread pool. The technique of thread pooling allows threads to be reused when thread creation cost is too high or creating an unbounded number of threads is a potential threat to the reliability of the system. Every thread that enters the pool expects to see an an object in its default, initialized form. However, when {{ThreadLocal}} objects are set from a thread which is subsequently made available for reuse, the reusing thread which takes its place may see the most recent state that was set by the previous thread instead of the expected, default state. \[[JPL 06|AA. Java References#JPL 06]\]

h2. Noncompliant Code Example

This noncompliant code example consists of an enumeration {{Day}} of days, a class {{Diary}} and a class {{DiaryPool}}. The class {{Diary}} uses a {{ThreadLocal}} variable to store thread-specific information, such as each thread's current day. The initial value of the current day is Monday, and this can be changed later by using the {{setDay()}} method. The thread also contains a thread-specific {{threadSpecificTask()}} instance method that performs a thread specific task. 

The class {{DiaryPool}} consists of two methods {{doSomething1()}} and {{doSomething2()}} that start a thread each, respectively. The method {{doSomething1()}} changes the initial (default) value of the day in the diary to Friday and invokes the {{threadSpecificTask()}} method. However, the method {{doSomething2()}} relies on the initial value of the day (Monday) in the diary and invokes the {{threadSpecificTask()}} method. The {{main()}} method creates one thread using {{doSomething1()}} and two more using {{doSomething2()}}.

{code:bgColor=#FFCCCC}
public enum Day {
  MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY, SATURDAY, SUNDAY;
}

public final class Diary {
  private static final ThreadLocal<Day> days =
  
    new ThreadLocal<Day>() {
      // Initialize to Monday 
      protected Day initialValue() {
        return Day.MONDAY;
      }
    };

  private static Day currentDay() {
    return days.get();
  }

  public static void setDay(Day newDay) {
    days.set(newDay);
  }
    
  // Performs some thread-specific task
  public void threadSpecificTask() {
    // Do task ...
    System.out.println("The current day is: " + currentDay());
  }
}


public final class DiaryPool {
  final int NoOfThreadsnumOfThreads = 2; // Maximum number of threads allowed in pool
  final Executor exec;
  final Diary diary;

  DiaryPool() {
    exec = (Executor) Executors.newFixedThreadPool(NoOfThreadsnumOfThreads);
    diary = new Diary();
  }

  public void doSomething1() {
    exec.execute(new Runnable() {
        @Override public void run() {
          Diarydiary.setDay(Day.FRIDAY);
          diary.threadSpecificTask();
        }
    });
  } 

  public void doSomething2() {
    exec.execute(new Runnable() {
        @Override public void run() {
          diary.threadSpecificTask();
       }
    });
  }

  public static void main(String[] args) {
    DiaryPool dp = new DiaryPool();
    dp.doSomething1(); // Thread 1, requires current day as Friday
    dp.doSomething2(); // Thread 2, requires current day as Monday
    dp.doSomething2(); // Thread 3, requires current day as Monday
  } 
}

{code}

This noncompliant code example sometimes prints: 

{code}
The current day is: FRIDAY
The current day is: FRIDAY
The current day is: MONDAY
{code}

The issue is that the {{DiaryPool}} class uses a thread pool to execute multiple threads. This allows threads to be reused when the pool is full. When this happens, the thread local state of a previous thread may be inherited by a new thread that has just begun execution. In this case, even though the threads that were started using {{doSomething2()}} are expected to see the current day as Monday, one of them inherits the day Friday from the first thread when the thread is reused. Changing the thread pool size to a larger size (more than 2) appears to fix the problem because it prints the expected state (Friday occurs only once):

{code}
The current day is: FRIDAY
The current day is: MONDAY
The current day is: MONDAY
{code}

This execution order may differ depending on thread scheduling, however, Friday occurs just once. Note that increasing the thread pool size from time to time is not a feasible option.

h2. Compliant Solution

The class {{Diary}} does not use a {{ThreadLocal}} object in this compliant solution. Also, the class {{DiaryPool}} uses local instances of class {{Diary}} within the methods {{doSomething1()}} and {{doSomething2()}}. The {{Day}} is uniquely maintained by each instance of the {{Diary}} class. As multiple threads are allowed to share a {{Diary}} instance, the {{day}} field is declared {{static}}. Creating two {{Diary}} instances in class {{DiaryPool}} allows the first thread to work with the object instance having the current day as Friday and the other two threads to work with the object instance with the current day as Monday.

{mc} The CS may need some work/explaining. Even if the noncompliant Diary class is used in the CS, it works just fine because different instances of Diary are created in DiaryPool as compared to the NCE {mc}

{code:bgColor=#ccccff}
class Diary {
  static Day day;

  Diary() {
    day = day.getInitialDay(Day.MONDAY); // Default	
  }

  private Day currentDay() {
    return day;
  }

  public void setDay(Day d) {
    day = d;
  }

  // Performs some thread-specific task
  public void threadSpecificTask() {
    // Do task ...
    System.out.println("The day is: " + currentDay());
  }
}
class DiaryPool {
  final int NoOfThreads = 2; // Maximum number of threads allowed in pool
  final Executor exec;

  DiaryPool() {
    exec = (Executor) Executors.newFixedThreadPool(NoOfThreads);
  }

  public void doSomething1() {
    final Diary diary = new Diary(); // First instance
    exec.execute(new Runnable() {
      public void run() {
        diary.setDay(Day.FRIDAY);
        diary.threadSpecificTask();
      }
    });
  } 

  public void doSomething2() {
    final Diary diary = new Diary(); // Second instance
    exec.execute(new Runnable(

public final class DiaryPool {
  final int numOfthreads = 3;
  // ...
}

public final class Diary {
  // ...
  public static void removeDay() {
    days.remove();
  }
}

public final class DiaryPool {
  // ...

  public void doSomething1() {
      exec.execute(new Runnable() {
        @Override public void run() {
          try {
            Diary.setDay(Day.FRIDAY);
            diary.threadSpecificTask();
          } finally {
            Diary.removeDay(); // Diary.setDay(Day.MONDAY) 
                               // can also be used
          }
        }
    });
  }

  // ...
}

class CustomThreadPoolExecutor extends ThreadPoolExecutor {
  public CustomThreadPoolExecutor(int corePoolSize,
      int maximumPoolSize, long keepAliveTime,
      TimeUnit unit, BlockingQueue<Runnable> workQueue) {
    super(corePoolSize, maximumPoolSize, keepAliveTime, 
          unit, workQueue);
  }

  @Override
  public void beforeExecute(Thread t, Runnable r) {
    if (t == null || r == null) {
      publicthrow voidnew runNullPointerException() {;
    }
    diaryDiary.threadSpecificTasksetDay(Day.MONDAY);
      }
  super.beforeExecute(t, r);
  });
  }

  public staticfinal void main(String[] args)class DiaryPool {
    DiaryPool dp = new// ...
  DiaryPool(); {
    dp.doSomething1(); // Thread 1, requires current day as Friday
exec = new CustomThreadPoolExecutor(NumOfthreads, NumOfthreads,
        dp.doSomething2(); // Thread 2, requires current day as Monday 
    dp.doSomething2(); // Thread 2, requires current day as Monday
  } 
}
{code}

As expected, this code correctly prints the following or some other order with Friday occurring just once:

{code}
The current day is: FRIDAY
The current day is: MONDAY
The current day is: MONDAY
{code}

Unmodifiable classes whose design incorporates {{ThreadLocal}} data should not be executed in thread pools.

h2. Risk Assessment

When objects of classes that use {{ThreadLocal}} data are executed in a thread pool by different threads, they may assume stale states, resulting in corrupt data.

|| Rule || Severity || Likelihood || Remediation Cost || Priority || Level ||
| CON27- J | high | probable | medium | {color:red}{*}P12{*}{color} | {color:red}{*}L1{*}{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+FIO38-J].

h2. References

\[[API 06|AA. Java References#API 06]\] class {{java.lang.ThreadLocal<T>}}
\[[JPL 06|AA. Java References#JPL 06]\] 14.13. ThreadLocal Variables

----
[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!|FIO36-J. Do not create multiple buffered wrappers on an InputStream]&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!|09. Input Output (FIO)]&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!|09. Input Output (FIO)]10, TimeUnit.SECONDS, new ArrayBlockingQueue<Runnable>(10));
    diary = new Diary();
  }
  // ...
}