Need

Ensuring proper synchronization and order of inputs to prevent race conditions

Context

• Usage of Ruby for Object-Oriented Programming
• Usage of ActiveRecord for Object-Relational Mapping (ORM) in database operations

Description

Non compliant code

        class Account < ActiveRecord::Base
  def transfer_to(target_account, amount)
    if self.balance >= amount
      sleep(rand(0..1)) # simulate network delay
      self.balance -= amount
      target_account.balance += amount
      self.save
      target_account.save
    else
      return "Insufficient balance"
    end
  end
end
        
        

The above code represents a method transfer_to in the Account class, which is responsible for transferring a certain amount from the current account to a target_account.

The vulnerability lies in the fact that this method is not atomic, i.e., it consists of multiple separate operations
- checking the balance, decrementing the balance of the current account, incrementing the balance of the target account, and saving both accounts.

If two threads call this method concurrently, a race condition can occur. For instance, if the balance is sufficient when checked in both threads, both threads might proceed to the decrement and increment operations. As a result, the balance of the current account might go below zero and the target account might receive double the amount.

This is because there is no mechanism to ensure that the sequence of operations is performed without interruption once it has started. This lack of synchronization and atomicity leads to the race condition vulnerability.

This vulnerability is further exacerbated by the sleep(rand(0..1)) line, which simulates a network delay. This artificial delay increases the window of opportunity for a race condition to occur.

Steps

• Use a locking mechanism to ensure atomicity of the transfer operation.
• Implement a transaction to wrap the transfer operation.
• Use database-level locks to prevent concurrent access to the accounts.
• Consider using optimistic locking to handle concurrent updates.
• Avoid using sleep or any delay mechanism that can introduce race conditions.
• Validate the input parameters to ensure they are in the expected format and order.

Compliant code

        class Account < ActiveRecord::Base
  def transfer_to(target_account, amount)
    ActiveRecord::Base.transaction do
      self.lock!
      target_account.lock!
      
      if self.balance >= amount
        self.balance -= amount
        target_account.balance += amount
        self.save!
        target_account.save!
      else
        return "Insufficient balance"
      end
    end
  end
end
        
        

The updated code uses ActiveRecord's transaction method to ensure that the operations within the block are atomic, meaning they are treated as a single unit of work. If any exception is raised within the block, all database changes made within the block are rolled back, and the exception is re-raised.

The lock! method is used to lock the current and target accounts for the duration of the transaction. This prevents other transactions from modifying the same data until the current transaction is complete, effectively preventing race conditions.

The save! method is used instead of save to ensure that the transaction is rolled back if there is an error saving the changes. The save! method raises an exception if the save fails, whereas the save method simply returns false.

The check for sufficient balance is performed within the transaction and after the accounts are locked. This ensures that the balance of the account cannot change between the check and the transfer operation.

This approach ensures that the transfer operation is atomic and isolated from other transactions, preventing race conditions and ensuring data integrity.

References

• 124. Race condition