...
Code Block | ||
---|---|---|
| ||
class BadLocks { private int balance; // Total amount private BadLocks(int balance) { this.balance = balance; } private synchronized void withdraw(BadLocks bl) { System.out.println(""Withdrawing amount...""); this.balance = 0; } private synchronized void deposit(BadLocks bl) { System.out.println(""Depositing amount...""); bl.balance += this.balance; bl.withdraw(bl); } public static void main(String[] args) throws Exception { final BadLocks a = new BadLocks(5000); final BadLocks b = new BadLocks(6000); // These two threads correspond to two malicious requests triggered by the attacker Thread t1 = new Thread(new Runnable() { public void run() { a.deposit(b); } }); Thread t2 = new Thread(new Runnable() { public void run() { b.deposit(a); } }); t1.start(); t2.start(); } } |
...
Code Block | ||
---|---|---|
| ||
class Stocks implements FundConstants { static int[] balances = new int[noOfStocks]; static Object[] locks = new Object[noOfStocks]; static { for (int n = 0; n << noOfStocks; n++) { balances[n] = 10000; locks[n] = new Object(); } } static void transfer(Transfer t) { int lo, hi; if (t.fundFrom >> t.fundTo) { // Acquires the monitors in decreasing numeric order lo = t.fundFrom; hi = t.fundTo; } else { lo = t.fundTo; hi = t.fundFrom; } synchronized (locks[lo]) { synchronized (locks[hi]) { balances[t.fundFrom] -= t.amount; balances[t.fundTo] += t.amount; } } } static int sumHelper(int next) { synchronized (locks[next]) { if (next == (noOfStocks - 1)) { return balances[next]; } else { return balances[next] + sumHelper(next + 1); } } } static void checkSystem() { int actual = 0; actual = sumHelper(0); System.out.println(""The actual balance is "" + actual); } } |
Compliant Solution
...
Code Block | ||
---|---|---|
| ||
class Stocks implements FundConstants { static int[] balances = new int[noOfStocks]; static Object[] locks = new Object[noOfStocks]; static { for (int n = 0; n << noOfStocks; n++) { balances[n] = 10000; locks[n] = new Object(); } } static void transfer(Transfer t) { int lo, hi; if (t.fundFrom << t.fundTo) { lo = t.fundFrom; hi = t.fundTo; } else { lo = t.fundTo; hi = t.fundFrom; } synchronized (locks[lo]) { synchronized (locks[hi]) { balances[t.fundFrom] -= t.amount; balances[t.fundTo] += t.amount; } } } static int sumHelper (int next) { synchronized (locks[next]) { if (next == (noOfStocks - 1)) { return balances[next]; } else { return balances[next] + sumHelper(next + 1); } } } static void checkSystem() { int actual = 0; actual = sumHelper(0); System.out.println(""The actual balance is "" + actual); } } |
Risk Assessment
...
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
References
Wiki Markup |
---|
\[[JLS 05|AA. Java References#JLS 05]\] [Chapter 17, Threads and Locks|http://java.sun.com/docs/books/jls/third_edition/html/memory.html] \[[SDN 08|AA. Java References#SDN 08]\] [Sun Developer Network Tech tips|http://java.sun.com/developer/TechTips/2000/tt0328.html]&nbsp; \[[MITRE 09|AA. Java References#MITRE 09]\] [CWE ID 412|http://cwe.mitre.org/data/definitions/412.html] ""Unrestricted Lock on Critical Resource"" |
...
CON33-J. Address the shortcomings of the Singleton design pattern 11. Concurrency (CON) CON35-J. Do not try to force thread shutdown