search

code-423n4 / 2021-06-gro-findings

0 stars 1 forks source link

All vaults must be private? #115

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

Vulnerability Details

The BaseVaultAdaptor's totalEstimatedAssets counts all assets in the vault as part of the assets. This suggests that all vaults must be private vaults that only the Gro protocol is allowed to invest in.

uint256 total = IERC20(token).balanceOf(address(this)).add(
  // full balance in vault is counted as part of protocol assets
  IERC20(token).balanceOf(address(vault))
);

Impact

If anyone can invest in the vaults, the total assets estimation might be inflated by unrelated deposits.

Recommended Mitigation Steps

Clarify if the assumption is correct or how it's supposed to work.

kitty-the-kat commented 3 years ago

deposit by function is only possible from the vault adapter, user might technically transfer tokens to the vault/vaultAdapter, which will be counted as part of the vault total Assets, but to no direct benefits to the user that transferred the assets

ghoul-sol commented 3 years ago

Directly transferring assets to the vault acts like a donation and doesn't have any side effects. Invalid.