DYAD - 0xlucky's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L56-L67

Vulnerability details

Impact

As mentioned in docs migration is going to happen from vaultManager to vaultManagerv2 , So during migration as mentioned in Deploy.V2.s.sol new WETH vault and new wSTETH vault will be deployed. And in that deploy script dyad is not deployed again. So considering that it can cause underflow in assetPrice and it will revert the function as in that numerator is calculated using above two values.

Also in observation 6.4 is mentioned that while calulating tvl, kerosene
vaults should not be consider. so only weth steth are remained.

Proof of Concept

"uint numerator = tvl - dyad.totalSupply();

here tvl will be totalvalue locked of vaults ( WETH and wSTETH ) which is calculated by balanceOf(address(vault)) , which will be very less when new vaults will be deployed of above 2 vaults.

But dyad contract address will be old only, dyad.totalSupply() would be very large.

And new deployed vaults would not be able to cope up with dyad totalsupply, so it will be less and underflow will occur and function will always revert, so we will not be to get assetprice from vault.kerosine.unbounded.sol .

Tools Used

Manual Review

Recommended Mitigation Steps

So instead of getting value of number of token from balanceOf(addres(vault)) , it should be mantained via state variable (internal accounting), so that value can be maintained while migrating too.

Assessed type

Under/Overflow