sherlock-audit / 2023-11-covalent-judging

3 stars 2 forks source link

ydlee - `getDelegatorTotalLocked` return wrong value. #101

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

ydlee

medium

getDelegatorTotalLocked return wrong value.

Summary

Function getDelegatorTotalLocked miscalculated the total locked value of a delegator.

Vulnerability Detail

The totalValueLocked is only added up for one validator, while it should be added up for all validators.

844:    function getDelegatorTotalLocked(address delegator) external view returns (uint128 totalValueLocked) {
845:        for (uint128 i = 0; i < validatorsN; i++) {
846:            Validator storage v = _validators[i];
847:            Staking storage s = v.stakings[delegator];
848:->          totalValueLocked = _sharesToTokens(s.shares, v.exchangeRate); // @audit use `+=` instead of `=`
849:            if (v._address == delegator) totalValueLocked += v.commissionAvailableToRedeem;
850:            Unstaking[] memory unstakings = v.unstakings[delegator];
851:            uint256 unstakingsN = unstakings.length;
852:            for (uint256 j = 0; j < unstakingsN; j++) {
853:                totalValueLocked += unstakings[j].amount;
854:            }
855:        }
856:
857:        return totalValueLocked;
858:    }

https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/OperationalStaking.sol#L844-L858

Impact

Function getDelegatorTotalLocked does not calculate the correct total locked value of a delegator, breaking the contract functionality and confusing offchain monitor systems.

Code Snippet

https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/OperationalStaking.sol#L844-L858

Tool used

Manual Review

Recommendation

Use "+=" instead of "=" at L848.

Duplicate of #35

sherlock-admin2 commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid according to sherlock rules on view function and for the fact that it has no secrity issue as that was done in the for loop

noslav commented 7 months ago

fixed by sum up delegations across all validators - sa35/sa101