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

12 stars 7 forks source link

When _mint is called in liquidate, the asset is not converted to shares #154

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The shares parameter passed to _mint is assets not shares, resulting in data confusion

Proof of Concept

_mint's code

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

_mint declaration

function _mint(address _receiver, uint256 _shares) internal virtual override;

Application of _amountOut

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

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

In short

_amountOut is assets, _mint needs shares, _amountOut needs to be converted to shares

Tools Used

Recommended Mitigation Steps

change from

_mint(_account, _amountOut);

to

_mint(_account, _convertToShares(_amountOut, Math.Rounding.Down));

Assessed type

Error

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #5

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory