veRWA - Tripathi's results

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L127 https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L240-L240

Vulnerability details

Impact

remove_gauge is a method present in GaugeController.sol which can be called only by governance. This method is prone to frontrun attack and due to that majority user's will be able to dissolve their votes for specific gauge.

All the users will call vote_for_gauge_weights with user_weight param zero and hence they will be able to use their votes on other gauge but weight of users who won't able to front run this tx , will be stuck in the removed gauge and no way to get them back

Proof of Concept

 function remove_gauge(address _gauge) external onlyGovernance {
        require(isValidGauge[_gauge], "Invalid gauge address");
        isValidGauge[_gauge] = false;
        _change_gauge_weight(_gauge, 0);
        emit GaugeRemoved(_gauge);
    }

this can be called by governance and which can be front run for the tx

 function vote_for_gauge_weights(address _gauge_addr, uint256 _user_weight) external {
     [....]
       power_used = power_used + new_slope.power - old_slope.power;
       require(power_used >= 0 && power_used <= 10_000, "Used too much power");

      [.....]
      }

some of all the weigh should not be more than 10000, hence they will front run initial tx to withdraw votes from the target gauge while other user's weight will be stuck here who won't able to front run the tx and their power used here will be stuck such that they won't able to retrieve it

Tools Used

Manual Testing

Recommended Mitigation Steps

SOrry i can't suggest any mitigation steps, yet need to dig more in the protocol to finally suggest a mitigation

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/main/src/VotingEscrow.sol#L487 https://github.com/code-423n4/2023-08-verwa/blob/main/src/VotingEscrow.sol#L572

Vulnerability details

Impact

VotingEscrow’s balanceOfAt and totalSupplyAt return their corresponding values for a given block. Because the balance and supply can vary within the same block, these functions can return different values when called on the current block.

Proof of Concept

These both function use a binary search to return their values associated with the block

    function _findBlockEpoch(uint256 _block, uint256 _maxEpoch) internal view returns (uint256) {
        // Binary search
        uint256 min = 0;
        uint256 max = _maxEpoch;
        // Will be always enough for 128-bit numbers
        for (uint256 i = 0; i < 128; i++) {
            if (min >= max) break;
            uint256 mid = (min + max + 1) / 2;
            if (pointHistory[mid].blk <= _block) {
                min = mid;
            } else {
                max = mid - 1;
            }
        }
        return min;
    }

if a block is contained in pointHistory, the latest one will be used.

Points on the current block can be added indefinitely in pointHistory. As a result , a user calling balanceOfAt or totalSupplyAt on the current block might not receive the latest value

Exploit Scenario

Alice creates a voting contract that relies on balanceOfAt and totalSupplyAt. Alice creates a vote using block.number as a snapshot and corrupts the quorum percentage

Tools Used

Manual

Recommended Mitigation Steps

these both function should not be called at current block . change require(_blockNumber <= block.number, "Only past block number"); to require(_blockNumber < block.number, "Only past block number");

Assessed type

Invalid Validation