Lines of code

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/pool/EthPool.sol#L30

Vulnerability details

The use of payable.transfer() is heavily frowned upon because it can lead to the locking of funds. The transfer() call requires that the recipient has a payable callback, only provides 2300 gas for its operation. This means the following cases can cause the transfer to fail:

The contract does not have a payable callback
The contract's payable callback spends more than 2300 gas (which is only enough to emit something)
The contract is called through a proxy which itself uses up the 2300 gas

Impact

Customers using non-EOA accounts with their positions cannot redeem() their LP tokens if the account has anything more than a basic receive() payable callback

Proof of Concept

redeem() calls _doTransferOut() using the msg.sender...

File: backd/contracts/pool/LiquidityPool.sol

567           _doTransferOut(payable(msg.sender), redeemUnderlying);

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/pool/LiquidityPool.sol#L567

which uses payable.transfer() to send back out Ether:

File: backd/contracts/pool/EthPool.sol

30           to.transfer(amount);

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/pool/EthPool.sol#L30

Note that there are a lot of other payable.transfer() instances, but none of them interact with users, only governance, vaults, and strategies, so I've split those off to my QA report

Tools Used

Code inspection

Recommended Mitigation Steps

Use msg.sender.call{value:x}() to send Ether

code-423n4 / 2022-04-backd-findings