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

12 stars 7 forks source link

Vault will lose its `_yieldFeeTotalSupply` without getting an equivalent amount of share tokens #397

Closed code423n4 closed 11 months ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

In Vault.sol/mintYieldFee function:

 _yieldFeeTotalSupply -= _shares;
_twabController.mint(_receiver, uint96(_shares));

Proof of Concept

Vault.sol/Line 398

File:pt-v5-vault/src/Vault.sol
Line 398: _yieldFeeTotalSupply -= _shares;

Vault.sol/Line 1123

File:pt-v5-vault/src/Vault.sol
Line 1123: _twabController.mint(_receiver, uint96(_shares));

Tools Used

Manual Testing.

Recommended Mitigation Steps

In mintYieldFee function : deduct the uint96(_shares) amount from _yieldFeeTotalSupply:

_yieldFeeTotalSupply -= uint96(_shares);

Assessed type

Math

c4-judge commented 1 year ago

Picodes changed the severity to 2 (Med Risk)

c4-sponsor commented 1 year ago

asselstine marked the issue as sponsor confirmed

c4-judge commented 11 months ago

Picodes marked the issue as duplicate of #458

c4-judge commented 11 months ago

Picodes marked the issue as satisfactory

PierrickGT commented 11 months ago

Fixed by safe casting to uint96 in the _mint function in this PR: https://github.com/GenerationSoftware/pt-v5-vault/pull/9/files#diff-97c974f5c3c03a0cfcbc52a5b8b9ae2196d535754ff2034e2903de1fec23508aR1130