0xblackskull - Missing access control in Admin::destroyInstance

[sherlock-admin3]

0xblackskull

high

Missing access control in Admin::destroyInstance

Summary

There is no access control in Admin::destroyInstance

Vulnerability Detail

When anyone trigger Admin::destroyInstance, it can remove any existing instance becoz lack of access control. destroyInstance() uses TokenSale::destroy, in TokenSale::destroy it check _onlyAdmin to authorize

 function _onlyAdmin() internal view {
        require(admin.hasRole(DEFAULT_ADMIN_ROLE, msg.sender) || msg.sender == address(admin), "TokenSale: Onlyadmin");
    }

in _onlyAdmin function it check msg.sender == address(admin) to authorize but admin is contract address of the admin contract which is pass in initialize function. Not completely check authorize who is calling it.

Impact

Any user can trigger destroyInstance with _instance value to remove it from sales.

Code Snippet

https://github.com/sherlock-audit/2024-03-zap-protocol/blob/main/zap-contracts-labs/contracts/Admin.sol#L138-L143 https://github.com/sherlock-audit/2024-03-zap-protocol/blob/main/zap-contracts-labs/contracts/TokenSale.sol#L186-L194

Tool used

Manual Review

Recommendation

add onlyRole(OPERATOR) in destroyInstance function

Duplicate of #118