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

2 stars 1 forks source link

Stale cvx price can be used while depositing #11

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/AfEth.sol#L138 https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategy.sol#L32

Vulnerability details

Impact

Stale cvx price can be used while depositing

Proof of Concept

When user deposits, then price of afEth token is calculated. It's needed to know how many tokens user will receieve.

This price consists of safEth price and vEth price.

This is how price is found for vEth. https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategy.sol#L31-L33

    function price() external view override returns (uint256) {
        return (cvxPerVotium() * ethPerCvx(false)) / 1e18;
    }

Here, ethPerCvx function is called and false is passed as param. This param tells function if it's needed to validate chainlink price. In case if it's false then there will be no validation and as result there is a risk that chainlink will return stale price. As user also provides slippage that actually doesn't mean that he will not be affected, it's still possible that wrong price will fit slippage.

Tools Used

VsCode

Recommended Mitigation Steps

Validate chainlink data in this case.

Assessed type

Error

elmutt commented 1 year ago

https://github.com/asymmetryfinance/afeth/pull/165 https://github.com/asymmetryfinance/afeth/pull/177

c4-judge commented 1 year ago

0xleastwood marked the issue as duplicate of #34

c4-judge commented 1 year ago

0xleastwood changed the severity to 3 (High Risk)

c4-judge commented 1 year ago

0xleastwood marked the issue as partial-50

0xleastwood commented 1 year ago

Partial credit because it is lacking additional information about impact.

c4-judge commented 1 year ago

0xleastwood marked the issue as satisfactory

c4-judge commented 1 year ago

0xleastwood removed the grade

c4-judge commented 1 year ago

0xleastwood marked the issue as partial-50

rvierdiiev commented 1 year ago

@0xleastwood with all respect, but impact is described at the top

When user deposits, then price of afEth token is calculated. It's needed to know how many tokens user will receieve.

so this means that wrong price will give wrong tokens amount

0xleastwood commented 1 year ago

That's not why I gave partial credit. You failed to detail the impact of such issue and under what situations this will affect deposits. Look at the primary issue and #28 to see what it would look like to contain sufficient context. And then look at #64 which I also gave partial credit to.

rvierdiiev commented 1 year ago

thanks, i will try to learn the difference and improve