Asymmetry contest - Vagner's results

Lines of code

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

Vulnerability details

Impact

Because of the way the underlyingValue is calculated in the stake() function an user who has all or a majority of the shares can manipulate the balances of the derivate contracts and steal the ether of any new user, if he has enough funds.

Proof of Concept

Bob is the first user of the protocol and stakes 3 ether with the stake() function, minting 3 safEth tokens and having 100% of the shares. Alice wants to use the protocol too and stakes 1 ether with the stake() function. Bob sees the transaction in the mempool and front-run the transaction, transferring an amount of wstEth/SfrxEth/Reth to the specific derivate contract, modifying the balanceOf that contract. The amount that needs to be sent to any of the contract can be calculated pretty easily by Bob since he needs to make it so the preDepositPrice is bigger than the totalStakeValueEth so when the mintAmount is calculated for Alice, it will get rounded to 0, because of the EVM rounding, creating a precision loss for Alice. For this specific case, Bob needs to transfer as much as 0.1 wstEth/SfrxEth/Reth to any of the 3 derivates contracts specifically, changing the underlyingValue and implicitly the preDepositPrice, which would be $(1e18 * 3.1)/3=> 1.06 * 1e18$. The totalStakeValueEth for Alice would be ~1, since she deposited 1 ether, so the mintAmount is $(11e18)/(1.061e18) => 0$ after the rounding. Alice funds will get transfered to the derivates contract but the amount of shares minted would be 0, so Bob can call the unstake() function, right after Alice stakes her ETH, with all of his shares, getting the balances of all the derivates contracts since he owns 100% of the shares. If Bob has enough funds to transfer to the contracts he can sandwich attack most of the users by just transferring before they stake and unstaking right after.

Tools Used

Manual review, Remix Debugger Tool

Recommended Mitigation Steps

The calculations that are done in the stake() function needs to be redone so there will not be such precision loss, or the business logic of the needs to be rethought so it will not use the balanceOf(address(this) since that can be easily manipulated by malicious users that have enough funds.