Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Edited by NavBot

...

Code Block
bgColor#FFcccc
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
bgColor#FFcccc
class Stocks implements FundConstants {
  static int[] balances = new int[noOfStocks];
  static Object[] locks = new Object[noOfStocks];
  static {
    for (int n = 0; n &lt;< noOfStocks; n++) {
      balances[n] = 10000;
      locks[n] = new Object();
    }
  }

  static void transfer(Transfer t) {
    int lo, hi;
    if (t.fundFrom &gt;> 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(&quot;"The actual balance is &quot;" + actual);
  }
}

Compliant Solution

...

Code Block
bgColor#ccccff
class Stocks implements FundConstants {
  static int[] balances = new int[noOfStocks];
  static Object[] locks = new Object[noOfStocks];
  
  static {
    for (int n = 0; n &lt;< noOfStocks; n++) {
      balances[n] = 10000;
      locks[n] = new Object();
    }
  }

  static void transfer(Transfer t) {
    int lo, hi;
    if (t.fundFrom &lt;< 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(&quot;"The actual balance is &quot;" + actual);
  }
}

Risk Assessment

...

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]&amp;nbsp;
\[[MITRE 09|AA. Java References#MITRE 09]\] [CWE ID 412|http://cwe.mitre.org/data/definitions/412.html] &quot;"Unrestricted Lock on Critical Resource&quot;"

...

CON33-J. Address the shortcomings of the Singleton design pattern&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;      11. Concurrency (CON)&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;      CON35-J. Do not try to force thread shutdown