peanuts - addProtocol() can override another protocol by accident

[sherlock-admin]

peanuts

medium

addProtocol() can override another protocol by accident

Summary

addProtocol() can override another protocol by accident.

Vulnerability Detail

When adding a Protocol and vault to the Controller, the DAO calls addProtocol() and passes in several important values like _vaultNumber and _name. These value's existence in the protocol are not checked and it may override an existing protocol.

  function addProtocol(
    string calldata _name,
    uint256 _vaultNumber,
    address _provider,
    address _protocolLPToken,
    address _underlying,
    address _govToken,
    uint256 _uScale
  ) external onlyDao returns (uint256) {
    uint256 protocolNumber = latestProtocolId[_vaultNumber];

    protocolNames[_vaultNumber][protocolNumber] = _name;
    protocolGovToken[_vaultNumber][protocolNumber] = _govToken;
    protocolInfo[_vaultNumber][protocolNumber] = ProtocolInfoS(
      _protocolLPToken,
      _provider,
      _underlying,
      _uScale
    );

Impact

Previous protocol and vault may be overridden.

Code Snippet

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Controller.sol#L146-L164

Tool used

Manual Review

Recommendation

Recommend adding checks that the latestProtocolId[_vaultNumber] does not exist beforehand.

  function addProtocol(
    string calldata _name,
    uint256 _vaultNumber,
    address _provider,
    address _protocolLPToken,
    address _underlying,
    address _govToken,
    uint256 _uScale
  ) external onlyDao returns (uint256) {
+   require(latestProtocolId[_vaultNumber] == 0, "Vault already created");
    uint256 protocolNumber = latestProtocolId[_vaultNumber];

    protocolNames[_vaultNumber][protocolNumber] = _name;
    protocolGovToken[_vaultNumber][protocolNumber] = _govToken;
    protocolInfo[_vaultNumber][protocolNumber] = ProtocolInfoS(
      _protocolLPToken,
      _provider,
      _underlying,
      _uScale
    );