0xBhumii - Possibility of Arithamatic Overflow or Underflow

[sherlock-admin]

0xBhumii

medium

Possibility of Arithamatic Overflow or Underflow

Summary

In OZVotingAdaptor contract function encodeVoteCallData use a parameter support which is uint8 which can cause overflow or underflow and cause the functions to revert.

Vulnerability Detail

In OZVotingAdaptor contract If the support parameter overflows in the encodeVoteCallData function, it can lead to unexpected behavior and potential contract failure. In Solidity versions 0.8 and above, arithmetic overflow or underflow results in a revert, preventing the contract state from being modified. Specifically, in this context, the support parameter is expected to be of type uint8, which has a maximum value of 255. If the support value exceeds this limit, the transaction will revert, indicating an overflow error.

Impact

Unintended Reverts and Inconsistent State:

Code Snippet

https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/adaptors/OZVotingAdaptor.sol#L42C2-L44C6

    function encodeVoteCallData(uint256 proposalId, uint8 support) external pure returns (bytes memory) {
        return abi.encode(proposalId, support);
    }  

POC

// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.8.23;

import {OZVotingAdaptorTest} from "./OZVotingAdaptor.t.sol";
import {IGovernor} from '@openzeppelin/contracts/governance/IGovernor.sol';
import {TestUtil} from 'test/lib/TestUtil.sol';

contract OverflowTest is OZVotingAdaptorTest {

    function testSupportOverflow() public {
        // Trigger the overflow by passing a value that's too large for uint8
        bytes memory params = abi.encode(10, 256);
        ozVotingAdaptor.vote(params);

        // Check for revert using a try/catch block
        try IGovernor(ozVotingAdaptor.governor()).castVote(1, 2) {} catch {
            require(false, "Expected revert did not occur");
        }
    }
}

here we can clearly see that EVM is reverting the transaction due to overflow

Tool used

Manual Review

Recommendation

To mitigate this potential issue, it's advisable to add a check to ensure that the support value does not exceed the expected range. This can be done by adding a require statement before encoding the data, as shown below:

require(support <= type(uint8).max, "Support value exceeds uint8 range");

This check ensures that the support value is within the valid range for a uint8, preventing overflow issues. The same check can be applied in the vote function or any other functions where the support parameter is used.

[nevillehuang]

Invalid, as mentioned in the issue, since solidity has an in-built check for overflow, an explicit check is not required

[sherlock-admin2]

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

pratraut commented:

'invalid as there is no real impact demonstrated due to such overflow with support value'