veRWA - 0xbrett8571's results

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/9a2e7be003bc1a77b3b87db31f3d5a1bcb48ed32/src/GaugeController.sol#L127-L132

Vulnerability details

Impact

When a gauge is removed through the remove_gauge function, you adjust the gauge weight to 0 without checking if there are active users who have already voted for this gauge. This might lead to unexpected results if users have already voted for the gauge before it's removed.

GaugeController.sol#L127-L131

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

The impact of this vulnerability can be severe:

1. Active users who have voted for the gauge being removed will lose their voting power associated with that gauge. This loss of incentives could lead to dissatisfaction among users who have actively participated in the system.

2. The lack of proper redistribution of voting power can lead to unexpected results in the governance and incentivization mechanisms of the contract. Users may receive fewer rewards than expected, disrupting the intended balance of the system.

3. Once the gauge is removed, it might become difficult to recover lost incentives and properly reallocate voting power to affected users, as the contract may not have mechanisms to retroactively address this issue.

Proof of Concept

Illustration of the potential issue with the remove_gauge function and the lack of consideration for users who have already voted for the gauge:

pragma solidity ^0.8.0;

contract GaugeController {
    mapping(address => bool) public isValidGauge;
    mapping(address => mapping(address => uint256)) public vote_user_power;

    function add_gauge(address _gauge) external {
        isValidGauge[_gauge] = true;
    }

    function remove_gauge(address _gauge) external {
        require(isValidGauge[_gauge], "Invalid gauge address");
        isValidGauge[_gauge] = false;
        // No logic to handle users who have voted for this gauge
    }

    function vote_for_gauge_weights(address _gauge_addr, uint256 _user_weight) external {
        require(_user_weight >= 0 && _user_weight <= 10_000, "Invalid user weight");
        require(isValidGauge[_gauge_addr], "Invalid gauge address");
        // Assume slope_ is calculated correctly
        // Calculate power_used correctly

        // Record user's vote
        vote_user_power[msg.sender][_gauge_addr] = _user_weight;
    }
}

contract User {
    GaugeController public gaugeController;
    
    constructor(address _gaugeController) {
        gaugeController = GaugeController(_gaugeController);
    }

    function voteForGauge(address _gauge, uint256 _weight) external {
        gaugeController.vote_for_gauge_weights(_gauge, _weight);
    }
}

contract Main {
    GaugeController public gaugeController;
    User public user;
    
    constructor() {
        gaugeController = new GaugeController();
        user = new User(address(gaugeController));
    }

    function simulateScenario() external {
        address gaugeA = address(0x123);
        address gaugeB = address(0x456);

        // Add gauges
        gaugeController.add_gauge(gaugeA);
        gaugeController.add_gauge(gaugeB);

        // User votes for Gauge A
        user.voteForGauge(gaugeA, 5000);

        // User votes for Gauge B
        user.voteForGauge(gaugeB, 3000);

        // Remove Gauge A without handling user votes
        gaugeController.remove_gauge(gaugeA);

        // Now, user's voting power for Gauge A is lost and not redistributed
        // This could lead to unexpected behavior and loss of incentives
    }
}

The simulateScenario function illustrates the issue. The user votes for both Gauge A and Gauge B. However, when Gauge A is removed through the remove_gauge function, there is no logic to handle the user's voting power for Gauge A. As a result, the user's voting power for Gauge A becomes lost or not properly redistributed, which could lead to unexpected behavior and loss of incentives.

Tools Used

VSCODE

Recommended Mitigation Steps

The logic error could be fixed by adding a check to the remove_gauge function to see if there are any active users who have already voted for the gauge. If there are any active users who have voted for the gauge, the function should not remove the gauge and should instead emit a warning to the users that their votes will no longer be counted.

function remove_gauge(address _gauge) external onlyGovernance {
        require(isValidGauge[_gauge], "Invalid gauge address");
        isValidGauge[_gauge] = false;
        bool hasVoters = false;
        for (uint256 i; i < voters[_gauge].length; ++i) {
            if (voters[_gauge][i].active) {
                hasVoters = true;
                break;
            }
        }
        if (hasVoters) {
            emit GaugeRemovedWarning(_gauge);
            return;
        }
        _change_gauge_weight(_gauge, 0);
        emit GaugeRemoved(_gauge);
    }

First checks to see if there are any active voters for the gauge. If there are no active voters, the function proceeds to remove the gauge and emit a "GaugeRemoved" event. If there are active voters, the function emits a "GaugeRemovedWarning" event and does not remove the gauge.

This would prevent the gauge from being removed if there are active users who have already voted for it. It would also warn the users that their votes will no longer be counted.

Assessed type

Governance

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/9a2e7be003bc1a77b3b87db31f3d5a1bcb48ed32/src/GaugeController.sol#L225 https://github.com/code-423n4/2023-08-verwa/blob/9a2e7be003bc1a77b3b87db31f3d5a1bcb48ed32/src/GaugeController.sol#L211

Vulnerability details

Impact

Lack of a buffer in the lock duration checking condition can lead to transaction failures for users whose lock duration is very close to the current epoch. This can result in a poor user experience and unnecessary gas expenses for failed transactions.

Proof of Concept

The condition require(lock_end > next_time, "Lock expires too soon"); in the vote_for_gauge_weights() function ensures that the user's lock doesn't expire too soon. However, you might want to consider adding a buffer to this condition to prevent users from submitting transactions that could fail due to small timing discrepancies.

The lock duration checking occurs at GaugeController.sol#L225

require(lock_end > next_time, "Lock expires too soon");

However, as mentioned, there is no buffer added to account for potential timing discrepancies, which could lead to failed transactions due to small variations in block timestamps.

Here's a scenario to illustrate the potential issue with the lock duration checking condition:

1. User Alice has a locked balance of CANTO tokens in the voting escrow contract.
2. Alice wants to vote for gauge weights using the vote_for_gauge_weights() function.
3. The vote_for_gauge_weights() function checks if Alice's lock duration (specified by lock_end) is greater than the current epoch (represented by next_time).
4. In this scenario, Alice's lock_end is exactly equal to the next_time (no buffer added).
5. Alice submits a transaction to vote for gauge weights.

Issue: Due to the lack of buffer in the lock duration checking condition, even if Alice's lock end is just a fraction of a second after the next_time, the condition lock_end > next_time would evaluate to false. This could lead to Alice's transaction failing with the error message "Lock expires too soon," even though her lock duration is effectively greater than the current epoch.

Blockchain transactions are processed in discrete blocks, and each block has a timestamp. If Alice's transaction is included in a block with a timestamp that falls after the next_time, even by a very small margin, the condition would evaluate to false, causing the transaction to fail. This is because the condition only checks if the lock end is strictly greater than the next_time.

Tools Used

VSCODE

Recommended Mitigation Steps

Consider adding a small buffer to the condition to account for timing discrepancies. For example, the condition can be modified to lock_end >= next_time + BUFFER, where BUFFER is a small duration (e.g., a few seconds) added to the next_time. This would provide a safety margin and ensure that users like Alice won't face transaction failures due to minor timing differences.

require(lock_end >= next_time + BUFFER, "Lock expires too soon");

Here, BUFFER represents a small duration (e.g., a few seconds) added to the next_time.

Assessed type

Solmate

The veRWA, Incentivization Primitive for Real World Assets on Canto is a well-designed and implemented system for incentivizing liquidity providers and voters on the Canto network. The system is based on a voting-escrow model, where users can lock up their CANTO tokens for five years in exchange for veCANTO tokens. veCANTO tokens can then be used to vote for lending markets, which are rewarded with CANTO tokens based on the amount of votes they receive. Users who provide liquidity to lending markets can also claim CANTO tokens based on their share of the liquidity in the market.

The veRWA system has a number of strengths, including:

• It is a simple and elegant system that is easy to understand and use.
• It is fair and impartial, as all users have an equal opportunity to earn rewards by locking up CANTO tokens and voting for lending markets.
• It is secure and robust, as it is based on well-tested and battle-tested contracts.
• It is scalable, as it can be easily adapted to support a large number of users and lending markets.

The veRWA system also has a few weaknesses, including:

• It is a long-term system, as users must lock up their CANTO tokens for five years in exchange for veCANTO tokens. This may be a barrier for some users who are looking for a more liquid investment.
• It is a centralized system, as the governance of the system is controlled by the Canto team. This could lead to potential conflicts of interest in the future.

Overall, the veRWA system is a well-designed and implemented system for incentivizing liquidity providers and voters on the Canto network. The system has a number of strengths, but it also has a few weaknesses. It will be important to monitor the system closely in the future to ensure that it continues to operate as intended.

Here are some additional comments for the judge to contextualize my findings:

• The approach taken in evaluating the codebase was to carefully review the code and documentation.
• The architecture of the system is well-designed, with a clear separation of concerns between the different contracts.
• The codebase quality is high, with few bugs and or security vulnerabilities.
• There are some centralization risks associated with the system, as the governance of the system is controlled by the Canto team. However, these risks can be mitigated by having a strong and transparent governance process.
• The mechanism review of the system is sound, and the system is designed to be fair and impartial.
• There are some systemic risks associated with the system, such as the risk of a black swan event that could destabilize the system. However, these risks can be mitigated by having a robust risk management plan in place.

Time spent:

22 hours