sherlock-audit / 2024-06-allora-judging

0 stars 0 forks source link

Kow - `GetAllReputersOutput`: If `listenedStakeFraction < minStakeFraction`, the differential will not be properly interpolated to ensure `listenedStakeFraction = minStakeFraction` #72

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 4 months ago

Kow

Medium

GetAllReputersOutput: If listenedStakeFraction < minStakeFraction, the differential will not be properly interpolated to ensure listenedStakeFraction = minStakeFraction

Summary

If listenedStakeFraction < minStakeFraction, the differential will not be properly interpolated to ensure listenedStakeFraction = minStakeFraction.

Vulnerability Detail

https://github.com/sherlock-audit/2024-06-allora/blob/4e1bc73db32873476f8b0a88945815d3978d931c/allora-chain/x/emissions/module/rewards/rewards_internal.go#L546-L574

        if listenedStakeFraction.Lt(minStakeFraction) {
            for l := range coefficients {
                coeffDiff, err := coefficients[l].Sub(oldCoefficients[l])
                if err != nil {
                    return nil, nil, err
                }
                ...
                if stakedFracDiff.IsZero() {
                    i = gradientDescentMaxIters
                } else {
                    coeffDiffTimesListenedDiff, err := coeffDiff.Mul(listenedDiff)
                    if err != nil {
                        return nil, nil, err
                    }
                    coefDiffTimesListenedDiffOverStakedFracDiff, err := coeffDiffTimesListenedDiff.Quo(stakedFracDiff)
                    if err != nil {
                        return nil, nil, err
                    }
                    coefficients[l], err = oldCoefficients[l].Add(coefDiffTimesListenedDiffOverStakedFracDiff)
                    if err != nil {
                        return nil, nil, err
                    }

In the whitepaper under equation 34, it is mentioned that whenever the fraction of the stake listened to falls below a minimum, the differential is interpolated so that the fraction is equal to the minimum. The corresponding logic is in the above snippet. The issue is coeffDiff calculates coefficients[i] - oldCoefficients[i], but at this point coefficients always contains the same values as oldCoefficients (since coefficients is copied into oldCoefficients at the start of the outer loop and neither is changed up to this point). Consequently, coeffDiff will always be 0 for all coefficients and coefficients will not change.

Impact

Fraction of stake listened to will not be corrected to the minimum if below the minimum, such that the adjusted stake(s) calculated will not be as intended resulting in incorrect distribution of rewards.

Code Snippet

https://github.com/sherlock-audit/2024-06-allora/blob/4e1bc73db32873476f8b0a88945815d3978d931c/allora-chain/x/emissions/module/rewards/rewards_internal.go#L546-L574

Tool used

Manual Review

Recommendation

Calculate coeffDiff = newCoefficients[i] - oldCoefficients[i] instead.

Duplicate of #93

sherlock-admin3 commented 4 months ago

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

0xmystery commented:

coeffDiff is always zero

sherlock-admin2 commented 3 months ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/allora-network/allora-chain/pull/516