hats-finance / Convergence-Finance---IBO-0x0e410e7af8e70fc5bffcdbfbdf1673ee7b3d0777

IBO, Vesting & Bond mecanism repo prepared for Hat finance audit competition
0 stars 0 forks source link

Centralization risk & Rug Pull: Owner Able to set any token address, crashing market #43

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @goheesheng Submission hash (on-chain): 0x446cb46b124596ec6673fcbed77a2bd521e243ed528d34555d24b30885cfcb7e Severity: high

Description: Description\ There is a Centralisation risk of the bridge where the onlyOwner of contracts/Oracles/CvgOracle.sol is able to modify the ERC20 token on the protocol to any arbitrary address. The owner can set any address to the token address, manipulating the market.

Attack Scenario\

Scenario 1: This would allow the admin role to change the address to one where they have an infinite supply, they could then call function withdrawOnlyExcess(uint256 _amount) external onlyOwner equal to the balance ofthis contract that is not in the vestingSchedulesTotalAmount. Even if without the full balance of this contract, the owner is still able to literally rug pull by setting the address to the infinite supply and then calling the withdraw function. It's also possible to set the address back thus "hiding" the rug pull.

Scenario 2: If the owner's wallet/private key is hacked or stolen, the owner is able to set to or create the malicious token address.

The hacked wallet is set to the malicious contract address. The malicious contract address has a million tokens. It can inflate the price and affect the price of the tokens or vice versa.

Scenario 1: If the malicious contract address has let's say 10 tokens, it is able to crash the protocol causing it to be inoperable. Users are unable to withdraw or redeem bonds causing a havoc.

Attachments

  1. Proof of Concept (PoC) File

    function setCvg(IERC20 _cvg) external onlyOwner {
        cvg = _cvg;
    }
    
    function setPresale(IPresaleCvgWl _newPresaleWl) external onlyOwner {
        presaleWl = _newPresaleWl;
    }
    
    function setPresaleSeed(IPresaleCvgSeed _newPresaleSeed) external onlyOwner {
        presaleSeed = _newPresaleSeed;
    }
    
    function setWhitelistTeam(address newWhitelistedTeam) external onlyOwner {
        whitelistedTeam = newWhitelistedTeam;
    }
    
    function setWhitelistDao(address newWhitelistedDao) external onlyOwner {
        whitelistedDao = newWhitelistedDao;
    }

    https://github.com/hats-finance/Convergence-Finance---IBO-0x0e410e7af8e70fc5bffcdbfbdf1673ee7b3d0777/blob/f43c5d9bc6b30c9f488e34836f09dc04d8f7361f/contracts/PresaleVesting/VestingCvg.sol#L115C1-L133C6

    function setCvgToken(IERC20Metadata _cvg) external onlyOwner {
        cvg = _cvg;
    }

    https://github.com/hats-finance/Convergence-Finance---IBO-0x0e410e7af8e70fc5bffcdbfbdf1673ee7b3d0777/blob/f43c5d9bc6b30c9f488e34836f09dc04d8f7361f/contracts/Oracles/CvgOracle.sol#L334C1-L336C6

  2. Mitigation\

Do create a Role-Based Access Control system if possible.

Without significant redesign, it is not possible to avoid the admin being able to rug-pull the protocol.

As a result, the recommendation is to set all admin functions behind either a timelocked DAO or at least a timelocked multisig contract.

shalbe-cvg commented 1 year ago

Hello, Thanks a lot for your attention.

You are referring to a centralization risk which is assumed on our side. However, to mitigate this kind of thing to happen, any changes impacting the protocol will be voted through our DAO and is subject to our multisig wallet. Since we are using a multisig wallet, no private key can be stolen or hacked to get access and use these functions.

We have so to consider this issue as Invalid.