Beedle - Oracle free perpetual lending

Summary

The deposit() function in Staking contract transfers the specified TKN amount from the msg.sender to the contract. The problem is the return value of this operation is never checked. Some tokens do not revert on transfer failure, but return false instead. This creates the possibility for the attacker to steal all of the TKNs from the Staking contract.

Vulnerability Details

The deposit() method looks like this:

The return value of the transferFrom method is never checked, so the following scenario is possible:

1. Attacker notices a Staking contract where 1 million TKNs have been deposited. The TKN is a token that does not revert on failure.

2. The attacker calls the deposit() function with the _amount of 1 million. The attacker does not have enough TKNs so the transfer will fail, but since the return value is never checked, the balances[msg.sender] will be updated to 1 million.

3. The attacker calls the withdraw() method, stealing the 1 milltion TKNs from the contract.

Impact

The attacker can steal all of the TKNs from the contract. This requires the TKN to be a token that does not revert on failure.

The unsafe ERC20 operations have been reported in the known issues section under low category, however this particular exploitation of them is a separate, high-risk issue.

Tools Used

Manual Review

Recommendations

Replace the transfer/transferFrom calls with the OpenZeppelin safeTransfer/safeTransferFrom.