code-423n4 / 2023-05-ajna-findings

2 stars 0 forks source link

Integer Overflow in executeExtraordinary Function. #428

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L67 https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L56-L67

Vulnerability details

Impact

The executeExtraordinary function casts a uint128 value to a uint256 value, which could lead to an integer overflow vulnerability. An attacker can provide a large uint128 value that exceeds the maximum value for uint256, causing the value to overflow and resulting in unintended behavior or security issues.

        uint256 tokensRequested = uint256(proposal.tokensRequested);

This line above casts a uint128 value proposal.tokensRequested to a uint256 value If the value of proposal.tokensRequested is greater than 2^128-1, which is the maximum value that can be represented by a uint128, then casting it to a uint256 will result in an incorrect value. This incorrect value could lead to issues down the line, especially if it is used in calculations or operations that rely on accurate values.

Proof of Concept

The vulnerable function

   function executeExtraordinary(
        address[] memory targets_,
        uint256[] memory values_,
        bytes[] memory calldatas_,
        bytes32 descriptionHash_
    ) external nonReentrant override returns (uint256 proposalId_) {
        proposalId_ = _hashProposal(targets_, values_, calldatas_, keccak256(abi.encode(DESCRIPTION_PREFIX_HASH_EXTRAORDINARY, descriptionHash_)));

        ExtraordinaryFundingProposal storage proposal = _extraordinaryFundingProposals[proposalId_];

        // since we are casting from uint128 to uint256, we can safely assume that the value will not overflow
        uint256 tokensRequested = uint256(proposal.tokensRequested);

The line uint256 tokensRequested = uint256(proposal.tokensRequested); casts a uint128 value proposal.tokensRequested to a uint256 value. If the value of proposal.tokensRequested is greater than 2^128-1, which is the maximum value that can be represented by a uint128, then casting it to a uint256 will result in an incorrect value, which can lead to issues down the line.

For example:

uint128 proposalTokensRequested = 2^128 - 1; // maximum value for uint128
uint256 tokensRequested = uint256(proposalTokensRequested); // casting uint128 to uint256

In this example, proposalTokensRequested has a value of 2^128 - 1, which is the maximum value that can be represented by a uint128. Casting it to a uint256 value will result in an incorrect value of 0, which can lead to unintended behavior or security issues.

Tools Used

Manual code review, vscode.

Recommended Mitigation Steps

Perform appropriate validation and error handling to prevent integer overflow vulnerabilities, it is recommended to validate that proposal.tokensRequested is within the range of values that can be safely cast to a uint256 value.

Alternatively, the contract could use a uint256 type for proposal.tokensRequested to avoid the need for casting altogether.

Assessed type

Under/Overflow

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid