Removing a nominee can brick the VoteWeighting.sol

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-05-olas/blob/3ce502ec8b475885b90668e617f3983cea3ae29f/governance/contracts/VoteWeighting.sol#L603-L612 https://github.com/code-423n4/2024-05-olas/blob/3ce502ec8b475885b90668e617f3983cea3ae29f/governance/contracts/VoteWeighting.sol#L232-L240

Vulnerability details

Impact

Slopes are not handled correctly when a nominee is removed. This can cause incorrect accounting for sum weights and lead to bricking the protocol in certain scenarios.

Proof of Concept

Points are accounted using bias, slope and end time. According to the formula, the bias is gradually decreased by its slope eventually reaching 0. In case the bias is suddenly reduced without handling/removing its slope, there will reach a time before the slope end time where the bias will drop below 0. pointsSum uses points to calculate the sum of individual nominee weights of at any time. The above case of bias dropping below 0 is handled as follows in _getSum:

            uint256 dBias = pt.slope * WEEK;
            if (pt.bias > dBias) {
                pt.bias -= dBias;
                uint256 dSlope = changesSum[t];
                pt.slope -= dSlope;
            } else {
                pt.bias = 0;
                pt.slope = 0;
            }

Whenever bias drops below 0 the slope is also made 0 without removing the slope end accounting in changesSum. This is what usually occurs when nominee is removed.

        // Set nominee weight to zero
        uint256 oldWeight = _getWeight(account, chainId);
        uint256 oldSum = _getSum();
        uint256 nextTime = (block.timestamp + WEEK) / WEEK * WEEK;
        pointsWeight[nomineeHash][nextTime].bias = 0;
        timeWeight[nomineeHash] = nextTime;

        // Account for the the sum weight change
        uint256 newSum = oldSum - oldWeight;
        pointsSum[nextTime].bias = newSum;
        timeSum = nextTime;

Afterwards, if a scenario occurs where the pt.bias regains a value greater than the previous dBias which is pt.slope * WEEK, the pt.bias > dBias part of the if condition executes which can cause it to revert if at any point changesSum[t] is greater than the current pt.slope due to underflow.

Once bias goes to 0, it can regain a value greater than the pt.slope * WEEK if another nominee is added or a a user votes for a nominee.

All this will lead to incorrect wight values and potential bricking of the protcool.

Tools Used

Manual code review

Recommended Mitigation Steps

Refactor the function to be like this.

            uint256 dBias = pt.slope * WEEK;
            if (pt.bias > dBias) {
                pt.bias -= dBias;
                uint256 dSlope = changesSum[t];
-           pt.slope -= d_slope;
+           if(pt.slope >= d.slope) {
+               pt.slope -= d_slope;
+           } else {
+               pt.slope = 0;
+           }
            } else {
                pt.bias = 0;
                pt.slope = 0;
            }

Assessed type

DoS

[c4-judge]

0xA5DF marked the issue as satisfactory

[c4-judge]

0xA5DF changed the severity to 2 (Med Risk)

[c4-judge]

0xA5DF changed the severity to 3 (High Risk)