code-423n4 / 2023-07-amphora-findings

3 stars 2 forks source link

Lack of Zero-Check for _cvxTotalRewards in _claimable Function Can Lead to Logical Errors #430

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-amphora/blob/daae020331404647c661ab534d20093c875483e1/core/solidity/contracts/core/AMPHClaimer.sol#L169

Vulnerability details

Impact

The _claimable function in the contract checks whether _crvTotalRewards or _amphBalance are zero, and if so, returns all zeros. However, there is no such check for _cvxTotalRewards. This omission can lead to a situation where the function returns non-zero _crvAmountToSend and _claimableAmph but zero _cvxAmountToSend.

This could potentially lead to undesired behavior or logical errors.

Proof of Concept

https://github.com/code-423n4/2023-07-amphora/blob/daae020331404647c661ab534d20093c875483e1/core/solidity/contracts/core/AMPHClaimer.sol#L169

The issue lies in the following line of the _claimable function:

// if amounts are zero, or AMPH balance is zero simply return all zeros if (_crvTotalRewards == 0 || _amphBalance == 0) return (0, 0, 0);

The function checks if _crvTotalRewards or _amphBalance are zero, but there is no check for _cvxTotalRewards. This can lead to a potential logical error if _cvxTotalRewards is zero.

Tools Used

Manual review/ChatGPT

Recommended Mitigation Steps

The condition should be modified to include a check for _cvxTotalRewards. The updated condition should look like this:

if (_cvxTotalRewards == 0 || _crvTotalRewards == 0 || _amphBalance == 0) return (0, 0, 0);

This will ensure that the function returns all zeros if _cvxTotalRewards, _crvTotalRewards, or _amphBalance are zero

Assessed type

Other

c4-pre-sort commented 1 year ago

minhquanym marked the issue as duplicate of #176

c4-judge commented 1 year ago

dmvt changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

dmvt marked the issue as grade-b