changeRebalanceConfigWithSignature will not work

Summary

changeRebalanceConfigWithSignature will not work which will lead to unable to maintain the stability, efficiency, and profitability protocol

Root Cause

There is a called down to hash function

    function changeRebalanceConfigWithSignature(
        RebalanceConfigChange calldata configChange,
        bytes calldata signature
    ) virtual external {
        _changeRebalanceConfigWithSignature(configChange, signature);
    }

/perennial-account/contracts/Controller.sol#L84

    function _changeRebalanceConfigWithSignature(RebalanceConfigChange calldata configChange, bytes calldata signature) internal {
        verifier.verifyRebalanceConfigChange(configChange, signature);
...
    }

/perennial-account/contracts/Controller.sol#L163

    function verifyRebalanceConfigChange(RebalanceConfigChange calldata change, bytes calldata signature)
        external
        validateAndCancel(change.action.common, signature)
    {
        if (!SignatureChecker.isValidSignatureNow(
            change.action.common.signer,
            _hashTypedDataV4(RebalanceConfigChangeLib.hash(change)),
            signature
        )) revert VerifierInvalidSignerError();
    }

account/contracts/AccountVerifier.sol#L70 Incorrect variable used in a function

    function hash(RebalanceConfigChange memory self) internal pure returns (bytes32) {
        bytes32[] memory encodedAddresses = new bytes32[](self.markets.length);
        bytes32[] memory encodedConfigs = new bytes32[](self.configs.length);

        // ensure consistent error for length mismatch
        if (self.markets.length != self.configs.length)
            revert IController.ControllerInvalidRebalanceConfigError();

        for (uint256 i = 0; i < self.markets.length; ++i) {
            encodedAddresses[i] = keccak256(abi.encode(self.markets[i])); // @audit hashed but not used below(anywhere)
            encodedConfigs[i] = RebalanceConfigLib.hash(self.configs[i]);
        }
        return keccak256(abi.encode(
            STRUCT_HASH,
            self.group,
            keccak256(abi.encodePacked(self.markets)), // @audit should be encodedAddresses
            keccak256(abi.encodePacked(encodedConfigs)),
            self.maxFee,
            ActionLib.hash(self.action)
        ));
    }

RebalanceConfigChange.sol#L52

Internal pre-conditions

markets array inside signature should be not empty, which seems like always non empty if protocol is live.

External pre-conditions

none

Attack Path

Admin/user tries to change rebalance config

Impact

It might lead to the inability to maintain the stability, efficiency, and profitability of protocol

PoC

In remix

pragma solidity >=0.7.0 <0.9.0;

import "hardhat/console.sol";

contract Owner{

    function showLog() public 
    {
        address[2] memory markets = [0x2082707c428E33009d233e00d56843aEF3FaC916, 0x4337001Fff419768e088Ce247456c1B892888084];
        bytes32[] memory encodedAddresses = new bytes32[](markets.length);
        for (uint256 i = 0; i < markets.length; ++i) {
            encodedAddresses[i] = keccak256(abi.encode(markets[i]));
        }
        console.logBytes32(keccak256(abi.encodePacked(markets)));
        console.logBytes32(keccak256(abi.encodePacked(encodedAddresses)));
    }
}

console.log:
0x91c59fff8b39368f96b15580746b90401d8c0cb1afe65431400232f4805ba32d
0xbc07560baa1063720814d68e888b5e0c08024c99ba3dd9ab50c827be5ea3460c

Mitigation

        return keccak256(abi.encode(
            STRUCT_HASH,
            self.group,
-            keccak256(abi.encodePacked(self.markets)),
+            keccak256(abi.encodePacked(encodedAddresses)),
            keccak256(abi.encodePacked(encodedConfigs)),
            self.maxFee,
            ActionLib.hash(self.action)
        ));

sherlock-audit / 2024-08-perennial-v2-update-3-judging

volodya - changeRebalanceConfigWithSignature will not work #1