OVL token shouldn't be available for substitution, needs to be set only once

[code423n4]

Handle

hyh

Vulnerability details

Impact

As OVL token is used as collateral for value storage and this way access to its mechanics equals access to the collateral funds.

A malicious Governor can switch OVL implementation to the one that steals funds on transfers, etc.

An observed possibility to alter core token implementation can also lead to reputational issues.

Proof of Concept

Governor role can instantly change OVL implementation: https://github.com/code-423n4/2021-11-overlay/blob/main/contracts/mothership/OverlayV1Mothership.sol#L75

Recommended Mitigation Steps

It's recommended to allow setting of OVL token implementation only once.

Now:

function setOVL (address _ovl) external onlyGovernor {
        ovl = _ovl;
}

To be:

function setOVL (address _ovl) external onlyGovernor {
        require(ovl == address(0), "setOVL: ovl already set");
        ovl = _ovl;
}

If the project roadmap requires implementation change in the future it is advised to add a Timelock to the functionality.

[mesozoic-technology]

I would say this is an incidental detail that would have been worked out as we get the contracts ready for their prime time deployment.

Whether or not we only set it once or allow it to be moved is a decision that we still need to consider finally as a team.

It could be we upgrade our token, after all.

[dmvt]

This is hypothetical at best. The attack described would require the governor to intentionally attack the users. There is no way it could happen by accident. The governor choosing to attack the protocol is highly unlikely. However, since it is possible and would cause a loss of user funds, the issue stands as a low severity.