Skip to content

Latest commit

 

History

History
126 lines (102 loc) · 4.46 KB

001.md

File metadata and controls

126 lines (102 loc) · 4.46 KB
Raw

Jolly Mauve Parakeet

Medium

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)
        ));