code-423n4 / 2024-03-pooltogether-findings

5 stars 4 forks source link

A variable indicating the number of vaults in existence should be there in the factory contract. #349

Closed c4-bot-1 closed 5 months ago

c4-bot-1 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVaultFactory.sol#L66

Vulnerability details

Impact

Due to security purposes, a variable (uint256) should exist as a counter for total number of vaults. It will be incremented when deployVault() is called and its value will be returned directly when totalVaults() will be called.

Proof of Concept

Tools Used

Manual review

Recommended Mitigation Steps

this can be used

uint256 vaultCounter = 0;

function deployVault(
      params
    ) external returns (PrizeVault) {
       ...
        allVaults.push(_vault);
        vaultCounter++;
       ...
    }

function totalVaults() external view returns (uint256) {
        return vaultCounter;
// this function also saves gas as we are reading the value from a state variable
    }

Assessed type

Other

c4-pre-sort commented 5 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 5 months ago

raymondfam marked the issue as primary issue

raymondfam commented 5 months ago

It's available via totalVaults():

https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVaultFactory.sol#L136-L138

c4-judge commented 5 months ago

hansfriese marked the issue as unsatisfactory: Invalid