Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: got rid of some debug print statements

...

Code Block
bgColor#FFcccc
class BadLocks {
  private int balance;  // Total amount
	 
  private BadLocks(int balance) {
    this.balance = balance;
  }

  private synchronized void withdraw() {
    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();
  }

  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#ccccff
private synchronized void deposit(BadLocks bl) {
  System.out.println("Depositing amount...");
  bl.balance += this.balance;
  withdraw(); // Call same instance's withdraw() method
}

...

Code Block
bgColor#ccccff
private volatile int balance;  // Total amount

//...

private void withdraw(BadLocks bl) {
  System.out.println("Withdrawing amount...");
  this.balance = 0; 
}

The withdraw() method does not require additional synchronization because it atomically makes the balance zero.

...

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 < noOfStocks; n++) {
      balances[n] = 10000;
      locks[n] = new Object();
    }
  }

  static void transfer(Transfer t) {
    int lo, hi;
    if (t.fundFrom > t.fundTo) { // Acquires the locks 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]) { // Acquires the locks in increasing numeric order
      if (next == (noOfStocks - 1)) {
        return balances[next];
      } else {
        return balances[next] + sumHelper(next + 1); // Increasing numeric order
      }
    }
  }

  static void checkSystem() {
    int actual = 0;
    actual = sumHelper(0);
    System.out.println("The actualBalance balance is " + 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 < noOfStocks; n++) {
      balances[n] = 10000;
      locks[n] = new Object();
    }
  }

  static void transfer(Transfer t) {
    int lo, hi;
    if (t.fundFrom < t.fundTo) { // Acquires the locks in increasing 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]) { // Acquires the locks in increasing numeric order
      if (next == (noOfStocks - 1)) {
        return balances[next];
      } else {
        return balances[next] + sumHelper(next + 1); // Increasing numeric order
      }
    }
  }

  static void checkSystem() {
    int actual = 0;
    actual = sumHelper(0);
    System.out.println("The actual balanceBalance is " + actual);
  }
}

Risk Assessment

...