VaultProxy's default owner can be changed in StaderConfig

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/9f1fc1217510b4f78e59c0fe854a3c2b64db963a/contracts/VaultProxy.sol#L35

Vulnerability details

Summary

The VaultProxy owner is set from StaderConfig.sol for each deployed proxy. However, in the case of an admin change in StaderConfig.sol, the onlyOwner functions won't be callable by the same account anymore.

Impact

If the admin of StaderConfig.sol is updated, every VaultProxy created before that change will require the manual intervention of the previous admin to change the owner in every VaultProxy.

If the owner was changed because it was compromised, that will also compromise the VaultProxy since the updateStaderConfig allows to upgrade the proxy at will.

Also, the effort of changing the owner in every VaultProxy will exhaust the network if there's the need of calling all the VaultProxies.

Tools Used

Manual Review

Recommended Mitigation Steps

• Instead of using owner in VaultProxy, use the StaderConfig.admin() in the onlyOwner modifier to keep every VaultProxy managed by the StaderConfig admin instead, allowing updating the owner for every vault in a single transaction

Assessed type

Access Control

[Picodes]

Correct but this isn't of high severity. It is more an interesting refactoring.

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)