ThresholdProxyAdmin Static Analysis Findings

TPA-01S: Inapplicacy of Checks-Effects-Interactions

Description:

The _acceptProposal function conducts the action of the proposal first prior to removing it from storage.

Example:

Copy
63function _acceptProposal(Proposal memory newProp) internal {
64    if(newProp.action == Action.Upgrade) {
65        if(newProp.data.length == 0) {
66            BaseAdminUpgradeabilityProxy(proxy).upgradeTo(newProp.target);
67        } else {
68            BaseAdminUpgradeabilityProxy(proxy).upgradeToAndCall(newProp.target, newProp.data);
69        }
70    } else if(newProp.action == Action.ChangeAdmin) {
71        BaseAdminUpgradeabilityProxy(proxy).changeAdmin(newProp.target);
72    }
73    _removeProposal(newProp);
74}

Recommendation:

We advise the order of actions to be reversed whereby the proposal is first removed and then actuated to conform to the Checks-Effects-Interactions security pattern given that a re-entrancy condition can materialize.

Alleviation:

The contract no longer exists in the codebase and this exhibit can be considered null.