static 宣言されたフィールドを更新し、かつ信頼できないコードから呼出し可能であるメソッドでは、static フィールドへのアクセスを同期しなくてはならない。たとえそのメソッドの使用規定でクライアントサイドロックを行うことを要求しても、信頼できないコードは同期を行わない可能性がある(不注意であるか悪意を持ってかを問わず)。static フィールドはすべてのクライアントが共有するため、信頼できないクライアントは、メソッドの使用規定に反して、適切なロックを行わないでフィールドにアクセスする恐れがある。
Joshua Bloch は次のように述べている。[Bloch 2008]
メソッドがstatic宣言されたフィールドを変更する場合、そのメソッドを利用するスレッドが通常は1つだけであったとしても、このフィールドへのアクセスは同期しなければならない。クライアント側で何らかの外的手法を使ってメソッドを同期しようとしても、それは不可能である。なぜなら、すべてのクライアントが同じように同期を行う保証はどこにもないからである。
プログラムが信頼できないコードとやりとりする場合、設計意図を明文化しても意味がない。攻撃者はそのような設計を無視するだけのことである。
違反コード
次の違反コードでは、staticなcounterフィールドへのアクセスを同期していない。
/* このクラスはスレッドセーフではない */ public final class CountHits { private static int counter; public void incrementCounter() { counter++; } }
クラス定義は、スレッドセーフを約束するクラスにのみ適用されるガイドライン「VNA02-J. 共有変数への複合操作のアトミック性を確保する」には適合している。しかし、このクラスには、パブリックアクセス可能なincrementCounter()メソッドによって変更される可変なstaticフィールドcounterが存在する。したがって、信頼できないコードが故意に同期を取らずにフィールドへアクセスすることが可能であるため、信頼できるクライアントコードがこのクラスを安全に使用することはできない。
適合コード
以下の適合コードでは、counterフィールドを保護するためにprivate static final宣言したロックを使用している。したがって、いかなる外部的な同期にも依存していない。またこの解決方法は、「LCK00-J. 信頼できないコードから使用されるクラスを同期するにはprivate finalロックオブジェクトを使用する」にも適合している。
/* このクラスはスレッドセーフである */ public final class CountHits { private static int counter; private static final Object lock = new Object(); public void incrementCounter() { synchronized (lock) { counter++; } } }
リスク評価
信頼できないコードによって変更されうるstatic宣言されたフィールドへのアクセスは、クラス内で同期しないと、間違った同期が行われるリスクが生じる。信頼できないコードは、たまたまあるいは悪意を持って、同期ポリシーを無視できるからである。
ルール | 深刻度 | 可能性 | 修正コスト | 優先度 | レベル |
---|---|---|---|---|---|
LCK05-J | 低 | 中 | 中 | P4 | L3 |
関連ガイドライン
MITRE CWE | CWE-820. Missing synchronization |
参考文献
[API 2006] | |
[Bloch 2008] | Item 67. Avoid excessive synchronization |
翻訳元
これは以下のページを翻訳したものです。
LCK05-J. Synchronize access to static fields that can be modified by untrusted code (revision 99)