Description

In underlying Definitive pools, the deployer can configure permissions during deployment. Importantly, a specific account is granted the DEFAULT_ADMIN_ROLE. In CoreAccessControl,

constructor(CoreAccessControlConfig memory cfg) {
    // admin
    _setupRole(DEFAULT_ADMIN_ROLE, cfg.admin);

    // definitive admin
    _setupRole(ROLE_DEFINITIVE_ADMIN, cfg.definitiveAdmin);
    _setRoleAdmin(ROLE_DEFINITIVE_ADMIN, ROLE_DEFINITIVE_ADMIN);

    // definitive
    for (uint256 i = 0; i < cfg.definitive.length; i++) {
        _setupRole(ROLE_DEFINITIVE, cfg.definitive[i]);
    }
    _setRoleAdmin(ROLE_DEFINITIVE, ROLE_DEFINITIVE_ADMIN);
    
    // clients - implicit role admin is DEFAULT_ADMIN_ROLE
    for (uint256 i = 0; i < cfg.client.length; i++) {
        _setupRole(ROLE_CLIENT, cfg.client[i]);
    }
}

In OpenZeppelin's AccessControl, the user with DEFAULT_ADMIN_ROLE has the ability to manage other roles.

Impact

This means that after deployment, the deployer is able to grant ROLE_CLIENT to other accounts. This allows them to steal funds by

1. granting that role to an account they control and

2. using that account to freely withdraw from the vault.

This would expose all users to potential loss of funds if the admin were ever compromised. It also requires unnecessary trust from users, which may discourage use of the protocol.

Recommendations

We recommend that Rainmaker set a smart contract or the vault manager itself as the sole owner of the vault. This may look like a system for transferring ownership during deployment.

Remediation

Rainmaker added documentation in commit , indicating that this risk is mitigated by granting underlying pool ownership to the Rainmaker multisig.