code-423n4 / 2023-07-pooltogether-findings

12 stars 7 forks source link

Malicious yield vault owners can manipulate the interaction between the vault and yield vault #447

Closed code423n4 closed 11 months ago

code423n4 commented 12 months ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L959

Vulnerability details

Impact

Malicious yield vault owners can steal depositor's assets.

Proof of Concept

yieldVault_ is the Address of the ERC4626 vault in which assets are deposited to generate yield. After tokens are deposited into the vault, the deposit function of yieldVault is called. Since anyone can create a yieldVault and anyone can create a vault, the deposit function called may not work as the intended ERC4626 function. If the yield vault creator is malicious, the deposit function can have code that transfers all assets from the depositor to the owner instead of minting shares.

      SafeERC20.safeTransferFrom(
        _asset,
        _caller,
        address(this),
        _assetsDeposit != 0 ? _assetsDeposit : _assets
      );
    }

    _yieldVault.deposit(_assets, address(this));
    _mint(_receiver, _shares);

The deposit function of the yieldVault can take the deposited assets from the user and route it to the malicious owner without minting any shares.

Tools Used

Manual Review

Recommended Mitigation Steps

Make sure that all yield vaults can be trusted. Have a whitelist to check that yield vault owners that intends to create the pool together vault has a proper vault set up first.

Assessed type

ERC4626

c4-judge commented 11 months ago

Picodes marked the issue as duplicate of #324

c4-judge commented 11 months ago

Picodes marked the issue as satisfactory