Compromised trader account can block admin from revoking access

The trading strategy contract for D2 defines two roles: ADMIN_ROLE and EXECUTOR_ROLE. The EXECUTOR_ROLE is intended for the active party responsible for executing trades on behalf of stakers, while the ADMIN_ROLE is a more passive role primarily reserved for emergency interventions.

The issue is in Vault::initialize, where args.trader is assigned both ADMIN_ROLE and EXECUTOR_ROLE:

s.grantRole(ADMIN_ROLE, args._owner);
s.grantRole(EXECUTOR_ROLE, args._owner);
s.grantRole(ADMIN_ROLE, args._trader);  // @audit admin given to trader as well
s.grantRole(EXECUTOR_ROLE, args._trader);

This setup introduces a security risk: if the trader's account is compromised, the attacker can revoke the owner's admin privileges, preventing them from removing the compromised trader from EXECUTOR_ROLE.

If the trader account is compromised, the protocol admin may be unable to revoke EXECUTOR_ROLE from the attacker before they can exploit their access. This could result in unauthorised or malicious trades, potentially harming the protocol and its stakeholders.

Do not assign ADMIN_ROLE to args.trader:

  s.grantRole(ADMIN_ROLE, args._owner);
  s.grantRole(EXECUTOR_ROLE, args._owner);
- s.grantRole(ADMIN_ROLE, args._trader);
  s.grantRole(EXECUTOR_ROLE, args._trader);

D2: Fixed in 614daaa.

Cyfrin: Verified.