Y2k Finance contest - R2's results

Lines of code

https://github.com/code-423n4/2022-09-y2k-finance/blob/2175c044af98509261e4147edeb48e1036773771/src/oracles/PegOracle.sol#L78

Vulnerability details

Impact

Errors in price calculation can lead to false positive unpegging.

1. Here you calculating nowPrice:

       if (price1 > price2) {
            nowPrice = (price2 * 10000) / price1;
        } else {
            nowPrice = (price1 * 10000) / price2;
        }

So the nowPrice will be less then 1e4

2. Then you are dividing nowPrice to 1e6:

nowPrice / 1000000

But if priceFeed1.decimals() == 18, nowPrice / 1000000 == 0 So then anyone can trigger unpeg, even if it's not true

3. Then in Controller.getLatestPrice() you are multiplying price to 10**(18-(priceFeed.decimals())) again It can lead to error too https://github.com/code-423n4/2022-09-y2k-finance/blob/2175c044af98509261e4147edeb48e1036773771/src/Controller.sol#L299

Tools Used

vs code

Recommended Mitigation Steps

Use base points instead of strikePrice, e.g. strikeDifferenceBsp = 200 - 2% difference between prices -> unpeg And fix point 1, 2 and 3 above It can help you to prevent math errors

Lines of code

https://github.com/code-423n4/2022-09-y2k-finance/blob/2175c044af98509261e4147edeb48e1036773771/src/Vault.sol#L203

Vulnerability details

Impact

In Vault.withdraw() you have modifier epochHasEnded(). But it's just checking time, no check if Controller already called triggerDepeg() or triggerEndEpoch()

So anyone can frontrun Controller transactions and withdraw their funds before without losses. For example risker can frontrun trx and withdraw funds in case of triggerDepeg() without funds loss

entitledAmount =
                    // @audit-ok high сначала делим - разве это не приведет к проблемам
                    amount.divWadDown(idFinalTVL[id]).mulDivDown(
                        idClaimTVL[id],
                        1 ether
                    ) +
                    amount;

So he will save his funds and get amount back

https://github.com/code-423n4/2022-09-y2k-finance/blob/2175c044af98509261e4147edeb48e1036773771/src/Vault.sol#L203

Tools Used

vs code

Recommended Mitigation Steps

Add explicit var, tells that epoch already ended by Controller. And only after that allow users to withdraw funds

Lines of code

https://github.com/code-423n4/2022-09-y2k-finance/blob/2175c044af98509261e4147edeb48e1036773771/src/Vault.sol#L167

Vulnerability details

Impact

You are sending tokens and then mint the same count of tokens But in case of tokens with fee (e.g. USDT) or deflationary tokens, that logic can lead to problems Because you will receive less tokens then sended

Tools Used

vs code

Recommended Mitigation Steps

Check token balances diff to check how much tokens you received

1. SemiFungibleVault._asset may be address(0) Add required check

1. Use entitledShares -= feeValue instead of entitledShares = entitledShares - feeValue

2. PegOracle.decimals is unused

3. No need to duplicate oracle1 and priceFeed1 in PegOracle. Because

priceFeed1 = AggregatorV3Interface(_oracle1)