code-423n4 / 2022-09-frax-findings

2 stars 1 forks source link

First xERC4626 deposit can break the share calculation. #378

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/transmissions11/solmate/blob/bff24e835192470ed38bf15dbed6084c2d723ace/src/mixins/ERC4626.sol#L133

Vulnerability details

Impact

New xERC4626 vault share price can be manipulated right after creation. Which give early depositor greater share portion of the vault during the first cycle. While deposit token also affected by rounding precision (due to the exploit showed in the POC) that always return less amount of share for user.

Proof of Concept

Solmate's function convertToShares follows the formula: assetDepositAmount * totalShareSupply / assetBalanceBeforeDeposit. https://github.com/transmissions11/solmate/blob/bff24e835192470ed38bf15dbed6084c2d723ace/src/mixins/ERC4626.sol#L133

The share price always return 1:1 with asset token. If everything work normally, share price will slowly increase with time to 1:2 or 1:10 as more rewards coming in. But right after xERC4626 contract creation, during first cycle, any user can deposit 1 share set totalSupply = 1. And transfer token to the vault to inflate totalAssets() before rewards kick in. This can inflate base share price as high as 1:1e18 early on, which force all subsequence deposit to use this share price as base.

How to test the exploit: Add this code to xERC4626Test.t.sol file to test: https://gist.github.com/CodingNameKiki/a53ce7946ff7e31f6b1c592c900b89f4

Results after the test: https://gist.github.com/CodingNameKiki/ef165677b51ddc571997cd23c1699c35

Tools Used

VS Code/Foundry

Recommended Mitigation Steps

This exploit is unique and only works if starting supply equal 0 or very small number and rewards cycle is very short. Or everyone withdraws, total share supply become 0.

This can be easily fix by making sure someone always deposited first so totalSupply become high enough, that this exploit become irrelevant. Unless in unlikely case someone made arbitrage bot watching the sfrxETH vault contract. Just force deposit early token during vault construction as last resort.

FortisFortuna commented 2 years ago

duplicate of https://github.com/code-423n4/2022-09-frax-findings/issues/247

OpenCoreCH commented 2 years ago

This was listed as a known issue (M-02) of xERC4626 in the contest description: https://code4rena.com/contests/2022-09-frax-ether-liquid-staking-contest