Sturdy contest - isamjay's results

Lines of code

https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/LidoVault.sol#L142

Vulnerability details

Impact

When the function '_withdrawFromYieldPool()' is called it sends Eth as collateral to the receiver. However, if the receiver is a contract that does not implement a fallback it will result in a failure but the contract continues without reverting because "receivedETHAmount" is returned before checking if the transaction was successful. The vault will assume the user has received the collateral even if it is not.

Proof of Concept

1. User tries to withdraw collateral to a smart contract.
2. The smart contract does not has a fallback function. The transaction should revert but doesn't.
3. The contract assumes the collateral for that user has been withdrawn creating a discrepancy.

Recommended Mitigation Steps

https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/LidoVault.sol#L142 Require statement should be moved before the return statement like below.

(bool sent, bytes memory data) = address(_to).call{value: receivedETHAmount}(''); require(sent, Errors.VT_COLLATERAL_WITHDRAW_INVALID); return receivedETHAmount;