Beedle - Oracle free perpetual lending

Summary

Reentrancy vulnerability in the borrow() function, which can result in draining all loanToken funds from the pool, and any other pool that the rogue borrower wishes to target in separate attack calls.

Vulnerability Details

The rogue borrower cannot loop through the for loop more than once for this attack, so to target more than one pool he needs to launch separate attacks each with their own independent borrow() function call and reentrancies. The result is draining of loanToken funds from one or more attacked pools.

Vulnerabilities:

• the borrow() function doesnt seem to have any reentrancy protection, as a public function callable by anyone.

• on L269 of Lender.sol, the transfer() function can trigger a callback into the Lender contract's borrow() function, if the ERC20 token contract enables callback functionality.

• transferFrom() executes after transfer(), so the rogue borrower can drain the pool funds via reentrancy before his collateral is transferred, for one loan only, to the pool.

Impact

Can result in draining all loanToken funds from the pool, and any other pool that the rogue borrower wishes to target in separate attack calls.

Tools Used

VSC, manual.

Recommendations

• add either a mutex lock or reentrancy modifier to the borrow() function

• If the transferFrom() was placed before the transfer(), it might have helped to mitigate this reentrancy attack.

• L276 should be placed before the transfer functions, in accordance with CEI pattern. Not that it matters much in this instance, but at least it would have recorded "duplicate" loan records (from reentrancy attack) into loans array(which I assume is possible), which would have helped with leaving evidence/trail of attack on protocol side.