Tadle

Summary

Transfers are performed before state updates which can lead to re-entrancy attacks and other unpredictable behaviors if an exception is thrown. This is because Solidity aims to be a "fail-safe" language, which means when an exception is thrown, all changes to the state from the current function call and all its sub-calls are reverted, apart from the payment of the gas fee. This aim to be fail-safe leads to interesting behaviors in the presence of re-entrancy attacks and related vulnerabilities.

Vulnerability Details

In DeliveryPlace.sol, functions closeBidOffer, closeBidTaker, settleAskMaker, and settleAskTaker update token balances before updating the contract states. This implies that in the event of a re-entrancy attack, the token balances could be manipulated while the contract state remains as is.

For example:

Above is an example of the affected code where token balance is added before updating the stock status.

Impact

the token balances could be manipulated while the contract state remains as is.

Tools Used

manual review

Recommendations

To remedy these findings, it's recommended to:

1. Always follow the Checks-Effects-Interactions Pattern to make your contract invulnerable to re-entrancy attacks and maintain state consistency.

2. Update any affected state variables before performing any transfer of tokens.

3. Consider making use of the re-entrancy guard modifier provided by OpenZeppelin's ReentrancyGuard contract if necessary.