Skip to content
This repository has been archived by the owner on Oct 1, 2023. It is now read-only.

Ruhum - Controller doesn't send treasury funds to the vault's treasury address #110

Open
sherlock-admin opened this issue Mar 27, 2023 · 5 comments
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin
Copy link
Contributor

Ruhum

high

Controller doesn't send treasury funds to the vault's treasury address

Summary

The Controller contract sends treasury funds to its own immutable treasury address instead of sending the funds to the one stored in the respective vault contract.

Vulnerability Detail

Each vault has a treasury address that is assigned on deployment which can also be updated through the factory contract:

    constructor(
        // ...
        address _treasury
    ) SemiFungibleVault(IERC20(_assetAddress), _name, _symbol, _tokenURI) {
        // ...
        treasury = _treasury;
        whitelistedAddresses[_treasury] = true;
    }

    function setTreasury(address _treasury) public onlyFactory {
        if (_treasury == address(0)) revert AddressZero();
        treasury = _treasury;
    }

But, the Controller, responsible for sending the fees to the treasury, uses the immutable treasury address that it was initialized with:

    constructor(
        // ...
        address _treasury
    ) {
        // ...
        treasury = _treasury;
    }

    // @audit just one example. Search for `treasury` in the Controller contract to find the others
    function triggerEndEpoch(uint256 _marketId, uint256 _epochId) public {
        // ...
        
        // send premium fees to treasury and remaining TVL to collateral vault
        premiumVault.sendTokens(_epochId, premiumFee, treasury);
        // strike price reached so collateral is entitled to collateralTVLAfterFee
        premiumVault.sendTokens(
            _epochId,
            premiumTVLAfterFee,
            address(collateralVault)
        );

        // ...
    }

Impact

It's not possible to have different treasury addresses for different vaults. It's also not possible to update the treasury address of a vault although it has a function to do that. Funds will always be sent to the address the Controller was initialized with.

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultV2.sol#L79
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultV2.sol#L265-L268

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol#L186
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol#L40

Tool used

Manual Review

Recommendation

The Controller should query the Vault to get the correct treasury address, e.g.:

collateralVault.sendTokens(_epochId, collateralFee, collateralVault.treasury());
@3xHarry
Copy link

3xHarry commented Apr 5, 2023

will use one location for the treasury address which will be on the factory.

@3xHarry 3xHarry added the Sponsor Confirmed The sponsor acknowledged this issue is valid label Apr 5, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Apr 11, 2023
@3xHarry
Copy link

3xHarry commented Apr 11, 2023

fixed in Y2K-Finance/Earthquake#137

@3xHarry 3xHarry added the Will Fix The sponsor confirmed this issue will be fixed label Apr 28, 2023
@IAm0x52
Copy link
Collaborator

IAm0x52 commented May 5, 2023

Needs additional changes. Controller still sends to it's immutable address and not treasury address on factory

@IAm0x52
Copy link
Collaborator

IAm0x52 commented May 8, 2023

Fix looks good. Controller has been updated to use treasury address from factory

@jacksanford1
Copy link

Note: 0x52 is referring to this specific commit in the last message:

Y2K-Finance/Earthquake@2721996

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

4 participants