Y2k Finance contest - ak1's results

Lines of code

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

Vulnerability details

Impact

When division operation is done before multiplication, the resultant can be a zero value output.

Proof of Concept

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

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

       entitledAmount = amount.divWadDown(idFinalTVL[id]).mulDivDown(
                      idClaimTVL[id],
                         1 ether
                    );

In above line of codes, division is performed before multilication.

Tools Used

Manual code review.

Recommended Mitigation Steps

It is recommended to multiply before doing division.

Lines of code

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

Vulnerability details

Impact

The calculated price1 could be negative or outdated one. This could affects the codes places wherever the latestRoundData is used to determine the price. one of the place is in Controller.sol#L261 - function getLatestPrice(address _token)

Proof of Concept

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

function latestRoundData()
    public
    view
    returns (
        uint80 roundID,
        int256 nowPrice,
        uint256 startedAt,
        uint256 timeStamp,
        uint80 answeredInRound
    )
{
    (
        uint80 roundID1,
        int256 price1,
        uint256 startedAt1,
        uint256 timeStamp1,
        uint80 answeredInRound1
    ) = priceFeed1.latestRoundData();


    int256 price2 = getOracle2_Price();


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


    int256 decimals10 = int256(10**(18 - priceFeed1.decimals()));
    nowPrice = nowPrice * decimals10;


    return (
        roundID1,
        nowPrice / 1000000,
        startedAt1,
        timeStamp1,
        answeredInRound1
    );
}

In above function, the price1 is directly get from the latestRoundData of the chainlink oracle.

The returned price value could be negative or outdated one.

Tools Used

VS code

Recommended Mitigation Steps

To calculate the price1, use the function getOracle1_Price that is already implemented. It has all the checks for negative and outdated. https://github.com/code-423n4/2022-09-y2k-finance/blob/2175c044af98509261e4147edeb48e1036773771/src/oracles/PegOracle.sol#L89

Lines of code

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

Vulnerability details

Impact

Due to buggy front end or user error, the deposited asset can be sent to zero address.

Proof of Concept

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

function deposit(
    uint256 id,
    uint256 assets,
    address receiver
)
    public
    override
    marketExists(id)
    epochHasNotStarted(id)
    nonReentrant
    returns (uint256 shares)

In above code snippet from deposit function, receiver will receive the minted share of tokens. By chance, if the receiver does not given as input, the minted tokens will end up in dead address (zero address).

Tools Used

Manual code review.

Recommended Mitigation Steps

Add validation check for receiver in the deposit function.

1. Get value once and use it wherever needed. https://github.com/code-423n4/2022-09-y2k-finance/blob/2175c044af98509261e4147edeb48e1036773771/src/Controller.sol#L96-L99 if( vault.strikePrice() < getLatestPrice(vault.tokenInsured()) ) revert PriceNotAtStrikePrice(getLatestPrice(vault.tokenInsured()));

call the getLatestPrice(vault.tokenInsured() once and use the result value in other places.

2. No need to initialize as zero in constructor. The default value itself is zero. https://github.com/code-423n4/2022-09-y2k-finance/blob/2175c044af98509261e4147edeb48e1036773771/src/VaultFactory.sol#L159