sherlock-audit / 2024-06-boost-aa-wallet-judging

3 stars 1 forks source link

Macho Mocha Donkey - Incorrect Revert Data Length in IncentiveBits Library #481

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 2 months ago

Macho Mocha Donkey

Low/Info

Incorrect Revert Data Length in IncentiveBits Library

Summary

The setOrThrow function in the IncentiveBits library uses an incorrect data length when reverting, potentially leading to inconsistent error handling and wasted gas.

Vulnerability Detail

In the setOrThrow function, when reverting due to an invalid incentive value, the code uses a revert data length of 0x24 (36 bytes). However, the actual data being written to memory is only 8 bytes: 4 bytes for the invalidSelector and 4 bytes for the incentive value. https://github.com/sherlock-audit/2024-06-boost-aa-wallet/blob/main/boost-protocol/packages/evm/contracts/validators/SignerValidator.sol#L115

mstore(0, invalidSelector)
mstore(4, incentive)
revert(0x00, 0x24)

This mismatch between the declared length and the actual data length can lead to inconsistent error handling, as the contract or external calls expecting a specific error format may misinterpret the additional, uninitialized memory.

Impact

The error data doesn't match the intended structure, which could complicate debugging and error analysis.

Code Snippet

function setOrThrow(IncentiveMap storage bitmap, bytes32 hash, uint256 incentive) internal {
    bytes4 invalidSelector = BoostError.IncentiveToBig.selector;
    bytes4 claimedSelector = BoostError.IncentiveClaimed.selector;
    /// @solidity memory-safe-assembly
    assembly {
        if gt(incentive, 7) {
            mstore(0, invalidSelector)
            mstore(4, incentive)
            revert(0x00, 0x24)  // Incorrect length
        }
        // ... (rest of the function)
    }
}

Tool used

Manual Review

Recommendation

Adjust the revert data length to match the actual data being written. If incentive is intended to be a uint256, the current length of 0x24 is correct. However, if incentive is meant to be a smaller value (e.g., uint8, given the check against 7), consider the following correction:

if gt(incentive, 7) {
    mstore(0, invalidSelector)
    mstore(4, and(incentive, 0xFF))  // Mask to ensure only 1 byte is used
    revert(0x00, 0x05)  // 4 bytes for selector + 1 byte for incentive
}