Y2k Finance contest - rvierdiiev's results

Lines of code

https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L345-L359 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L136 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L152 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L187-L190

Vulnerability details

Impact

VaultFactory allows admin to change controller for marketId(hedge and risk vaults) using VaultFactory.changeController. This method then set controller to both vaults. This address is important for Vault contract as it allows to call different functions.

VaultFactory take care about different pair vaults through indexVaults mapping. Controller can get info about pairs vaults only through the correct VaultFactory that is provided to Controller in constructor.

It's possible that VaultFactory.changeController will set controller whose vaultFactory field is not equal to current VaultFactory. That means that when Controller.triggerDepeg or Controller.triggerEndEpoch will be called they will not be able to find the market. So current controller will not be able to call hedge and risk vaults.

Proof of Concept

This is how the controller is set to vaults. https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L345-L359

Controller depends on VaultFactory to find vault for market. https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L136 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L152

Tools Used

Recommended Mitigation Steps

Use same check as you used in VaultFactory.createNewMarket https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L187-L190

1. Remove variable as it is nod used. https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L11
2. Modifier is not used. https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L73-L77
3. Conditions with assertions to true or false are confusing. Use !vault.idExists(epochEnd) instead. https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L93 Use if(!insrVault.idExists(epochEnd) || !riskVault.idExists(epochEnd)) instead https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L211 Use !idExists[id] instead https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L80 Use !isApprovedForAll(owner, receiver) instead https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L217 Use if(idExists[epochEnd]) instead https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L314
4. Check address param for 0 address. https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/SemiFungibleVault.sol#L70 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/SemiFungibleVault.sol#L96 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L80 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L365 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L312
5. Add event description. https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L113

1. Cache getLatestPrice(vault.tokenInsured()) into variable as you use it int 2 places. https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L96-L99
2. Use external modifier instead of public if you don’t call method from a contract to save deployment gas. https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L148 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L217 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L277 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L287 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L295 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L307 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L336 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L350 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L360 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L439 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L186 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L180 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L253 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L295 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L308 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L327 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L345 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L366 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/VaultFactory.sol#L385
3. Use if (answer > 0) to save gas. Do not create Boolean variable. https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L277-L280
4. Use > instead of !=0 to save gas. https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L157 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L159 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L215 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Controller.sol#L217
5. Use ++i instead of i++ for loops. Also cache epochsLength() into variable. https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L443