Lines of code

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

Vulnerability details

Impact

EthPool sends out native tokens via payable.transfer call. This is unsafe as transfer has hard coded gas budget and can fail when the to is a smart contract. Such transactions will fail for smart contract users which don't fit to 2300 gas stipend transfer have.

As user-facing redeem is the only way to retrieve underlying asset by burning LP tokens, this means that such accounts will be unable to retrieve underlying at all, having them frozen within the system. Setting severity to high as funds are lost for such users.

Proof of Concept

EthPool's _doTransferOut uses payable(to).transfer(amount):

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

EthPool._doTransferOut is called with an arbitrary 'to', which is 'msg.sender', by user-facing LiquidityPool.redeem():

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

Unlike the case where Vault/Pool/Strategy is a receiver, which is controllable by having low-gas receive function (this a separate lower severity issue), redeem() proper execution here cannot be controlled as receiver is an arbitrary msg.sender.

References

The issues with transfer() are outlined here:

https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

Recommended Mitigation Steps

As redeem isn't nonReentrant, the replacing of transfer() should be coupled with the introduction of non-reentrancy feature.

After that using low-level call.value(amount) with the corresponding result check or using the OpenZeppelin Address.sendValue is advised:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/Address.sol#L60

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