code-423n4 / 2023-01-astaria-findings

5 stars 2 forks source link

Anyone can deploy a public Vault #604

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/AstariaRouter.sol#L544

Vulnerability details

Impact

A malicious user can deploy a Vault and for example, set very high fees.

Proof of Concept

The vaults are deployed by AstariaRouter contract. Normally only whitelisted strategist can deploy public Vaults but there´s no guard in the deploy function newPublicVault()

unction newPublicVault(  // @audit vault fee puede ser lo grande que uno quiere
    uint256 epochLength,
    address delegate,
    address underlying,
    uint256 vaultFee,
    bool allowListEnabled,
    address[] calldata allowList,
    uint256 depositCap
  ) public whenNotPaused returns (address) {   
    RouterStorage storage s = _loadRouterSlot();
    if (s.minEpochLength > epochLength) {
      revert IPublicVault.InvalidState(
        IPublicVault.InvalidStates.EPOCH_TOO_LOW
      );
    }
    if (s.maxEpochLength < epochLength) {
      revert IPublicVault.InvalidState(
        IPublicVault.InvalidStates.EPOCH_TOO_HIGH
      );
    }
    return
      _newVault(
        s,
        underlying,
        epochLength,
        delegate,
        vaultFee,
        allowListEnabled,
        allowList,
        depositCap
      );
  }

Tools Used

manual review

Recommended Mitigation Steps

you can use a mapping address -> bool isWhitelisted and a modifier OnlyWhitelistedStrategist() .

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #58

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

Picodes marked the issue as grade-c