WstEth.withdraw() improper implementation of slippage check

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L56-L67

Vulnerability details

Impact

In the current implementation of withdraw(), the _amount is not controlled by minOut.

Impact: Users can get rekt.

Proof of Concept

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L56-L67

function withdraw(uint256 _amount) external onlyOwner {
        IWStETH(WST_ETH).unwrap(_amount);
        uint256 stEthBal = IERC20(STETH_TOKEN).balanceOf(address(this));
        IERC20(STETH_TOKEN).approve(LIDO_CRV_POOL, stEthBal);
        uint256 minOut = (stEthBal * (10 ** 18 - maxSlippage)) / 10 ** 18;
        IStEthEthPool(LIDO_CRV_POOL).exchange(1, 0, stEthBal, minOut);
        // solhint-disable-next-line
        (bool sent, ) = address(msg.sender).call{value: address(this).balance}(
            ""
        );
        require(sent, "Failed to send Ether");
    }

Tools Used

Manual review

Recommended Mitigation Steps

Consider implementing the minOut check on line 57 as below:

require(_amount >= minOut, "Slippage Check");

also in SfrxEth.withdraw()

[c4-pre-sort]

0xSorryNotSorry marked the issue as low quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[elmutt]

minOut is talking about an eth value. _amount is a derivative value. we use minOut in the exchange to eth

[c4-sponsor]

elmutt marked the issue as sponsor disputed

[c4-judge]

Picodes marked the issue as unsatisfactory:
Invalid