code-423n4/2023-03-asymmetry-findings

All the FRX_ETH tokens of SfrxEth contract can be drained by a malicious user.

Closed this issue · 8 comments

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/SfrxEth.sol#L60

Vulnerability details

Impact

The impact of this finding is severe, as it can result in the complete loss of FRX_ETH tokens held by the SfrxEth contract. This could lead to a significant financial loss for the contract and its users.

Proof of Concept

For demonstration purpose, Alice is sending 9 FRX_ETH to the SfrxEth (or in real world example admins of protocol want to transfer STETH for rebalancing the pools) immediately after that anybody who call withdraw will be able to drain the whole STETH token inside SfrxEth contract. For this example hacked deposits 0.5 ether and withdraw in in one transaction and will gain initial 0.5 ether + 9 ether as native ether.

    function testSfrxEth() public {
        // alice calls
        vm.startPrank(alice);
        IERC20(SFRX_ETH_ADDRESS).transfer(address(sfrxEth), IERC20(STETH_TOKEN).balanceOf(alice));
        vm.stopPrank();

        // hacker calls
        safEth.stake{value : 0.5 ether}();
        safEth.unstake(0.5 ether);
    }

Tools Used

Manual, Foundry

Recommended Mitigation Steps

You can revise unstake function of SafEth to only return the correct amount belong to user or revising the withdraw function of SrfxEth contract.

0xSorryNotSorry marked the issue as low quality report

0xSorryNotSorry marked the issue as primary issue

Removing low quality report on behalf of the Lookout, so that this issue can still be reviewed by the sponsor.

toshiSat marked the issue as disagree with severity

toshiSat requested judge review

Yes, users who send erc20 tokens directly to contracts will lose funds

toshiSat marked the issue as sponsor disputed

Picodes marked the issue as unsatisfactory:
Overinflated severity