009.md

Tangy Hotpink Monkey

Medium

Improper Input Validation in _raiseKeeperFee Function

Summary

The _raiseKeeperFee function in the Controller_Arbitrum contract lacks proper input validation for the data parameter, which can lead to unexpected behavior, incorrect fee calculations, and potential denial of service due to excessive gas consumption. This vulnerability arises from the absence of checks on the format and content of the input data, which is expected to contain encoded information about the collateral account address and user-specified maximum fee.

Vulnerability Detail

Issues:

• The function does not validate the length or contents of the data parameter.
• It assumes data contains a valid address and a UFixed6-encoded maximum cost without verification.
• The lack of checks can lead to incorrect decoding, which causes the function to operate on invalid data.

    function _raiseKeeperFee(
        UFixed18 amount,
@=>     bytes memory data
    ) internal override(Controller_Incentivized, Kept) returns (UFixed18 raisedKeeperFee) {
        return Controller_Incentivized._raiseKeeperFee(amount, data);
    }

Technical Explanation:

• The data parameter is used to extract the address and cost limits.
• Without validation, malformed data can bypass expected logic.
• For example, if data is shorter than expected, the decode process will read uninitialized memory or an invalid value.

Impact

• Malformed data can lead to incorrect fee calculations, potentially allowing attackers to manipulate the fee to their advantage.
• Invalid data can cause the function to consume excessive gas, leading to potential denial of service by exhausting the gas limit.

Code Snippet

https://github.com/sherlock-audit/2024-08-perennial-v2-update-3/blob/main/perennial-v2/packages/perennial-account/contracts/Controller_Arbitrum.sol#L32-L37

Tool used

Manual Review

Recommendation

• Implement strict validation on the data parameter to ensure it matches the expected format and length.
• Add error handling mechanisms to gracefully manage decoding failures or invalid data scenarios.

    function _raiseKeeperFee(UFixed18 amount, bytes memory data) internal returns (UFixed18 raisedKeeperFee) {
        // Make sure the data length is as expected.
        require(data.length == 32, "Invalid data length");

        // Decode data into addresses and maximum costs
        (address collateralAccount, uint256 maxFee) = abi.decode(data, (address, uint256));

        // Address validation
        require(collateralAccount != address(0), "Invalid collateral account address");

        // Maximum cost validation
        require(maxFee > 0, "Invalid max fee");

        // Do the logic to increase the guard fee
        // For example, make sure the amount does not exceed the maxFee.
        if (amount.unwrap() > maxFee) {
            raisedKeeperFee = UFixed18.wrap(maxFee);
        } else {
            raisedKeeperFee = amount;
        }

        // Logic of transfer of funds or other compensation
        // ...

        return raisedKeeperFee;
    }