nobody2018 - ChangeTreasury does not correctly set the new treasury address and does not remove the old treasury address from whitelist

[sherlock-admin]

nobody2018

medium

ChangeTreasury does not correctly set the new treasury address and does not remove the old treasury address from whitelist

Summary

Currently, the treasury of VaultV2 is used to receive deposit fees. When calling changeTreasury to change the treasury address, the function does not correctly set the new treasury address and does not remove the old treasury address from whitelist.

Vulnerability Detail

The changeTreasury incorrectly passed the treasury address of VaultFactoryV2 to the VaultV2.setTreasury function. Let's take a look at the [code](https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultFactoryV2.sol#L228-L246):

function changeTreasury(uint256 _marketId, address _treasury)
        public
        onlyTimeLocker
    {
        if (_treasury == address(0)) revert AddressZero();

        address[2] memory vaults = marketIdToVaults[_marketId];

        if (vaults[0] == address(0) || vaults[1] == address(0)) {
            revert MarketDoesNotExist(_marketId);
        }

        IVaultV2(vaults[0]).whiteListAddress(_treasury);
        IVaultV2(vaults[1]).whiteListAddress(_treasury);
        IVaultV2(vaults[0]).setTreasury(treasury);	//should be _treasury
        IVaultV2(vaults[1]).setTreasury(treasury);	//should be _treasury

        emit AddressWhitelisted(_treasury, _marketId);
    }

To maintain consistency, when changing treasury, the old treasury should be removed from whitelist to prevent accidental transfer of funds to the old treasury. If the old treasury is abandoned, the funds will be lost.

Impact

Setting Treasury errors may cause funds loss.

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultFactoryV2.sol#L228-L246

Tool used

Manual Review

Recommendation

--- a/Earthquake/src/v2/VaultFactoryV2.sol
+++ b/Earthquake/src/v2/VaultFactoryV2.sol
@@ -239,8 +239,11 @@ contract VaultFactoryV2 is Ownable {
 
         IVaultV2(vaults[0]).whiteListAddress(_treasury);
         IVaultV2(vaults[1]).whiteListAddress(_treasury);
-        IVaultV2(vaults[0]).setTreasury(treasury);
-        IVaultV2(vaults[1]).setTreasury(treasury);
+        //remove old treasury from whitelist
+        IVaultV2(vaults[0]).whiteListAddress(IVaultV2(vaults[0]).treasury());
+        IVaultV2(vaults[1]).whiteListAddress(IVaultV2(vaults[1]).treasury());
+        IVaultV2(vaults[0]).setTreasury(_treasury);
+        IVaultV2(vaults[1]).setTreasury(_treasury);
 
         emit AddressWhitelisted(_treasury, _marketId);
     }

Duplicate of #208

[securitygrid]

Escalate for 10 USDC
The report is a duplicate of #208. The title is "ChangeTreasury does not correctly set the new treasury address and does not remove the old treasury address from whitelist". The Recommendation is the same as #208.

[sherlock-admin]

Escalate for 10 USDC
The report is a duplicate of #208. The title is "ChangeTreasury does not correctly set the new treasury address and does not remove the old treasury address from whitelist". The Recommendation is the same as #208.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[hrishibhat]

Escalation accepted

Valid duplicate of #208

[sherlock-admin]

Escalation accepted

Valid duplicate of #208

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.