[TD-1396] Immutable Signed Zone v2 Threat Model #206
Conversation
|
|
||
| ## Threat Model Scope | ||
|
|
||
| The threat model is limited to the following Solidity files at GitHash [TBD]: |
There was a problem hiding this comment.
Add the githash and link to browse files at that githash. That is needed, so people can understand what the threat model was based on
There was a problem hiding this comment.
Will do once we've actioned code changes below.
There was a problem hiding this comment.
Added links to githash
|
|
||
| The threat model is limited to the following Solidity files at GitHash [TBD]: | ||
|
|
||
| * [ImmutableSignedZoneV2.sol [TBD]]() |
There was a problem hiding this comment.
Please put in the links.
There was a problem hiding this comment.
Will do once we've actioned code changes below.
|
|
||
| * Default Admin | ||
| * Creates and removes other administrators | ||
| * First admin is assigned to the `address owner` param on the `constructor` |
There was a problem hiding this comment.
Is this owner related to linking the contract to an account in Immutable Hub? If so, it would be good to switch to what is being done in Assets / what they are moving towards, where the DEFAULT_ADMIN_ROLE just deals with role administration, and there is a HUB_OWNER_ROLE.
There was a problem hiding this comment.
Our contract is intended to be deployed and managed by Immutable exclusively. Hub is not involved in this management.
There was a problem hiding this comment.
The owner will be set to the multi-sig
| * Default Admin | ||
| * Creates and removes other administrators | ||
| * First admin is assigned to the `address owner` param on the `constructor` | ||
| * Call the following configuration functions: |
There was a problem hiding this comment.
These tasks should be controlled by a separate admin to the DEFAULT_ADMIN_ROLE
There was a problem hiding this comment.
Switched to a new ZONE_MANAGER_ROLE
|
|
||
| * Grant can grant administrator roles to any account, including the `DEFAULT_ADMIN` role | ||
| * Revoke `DEFAULT_ADMIN` role from any account | ||
| * Renounce the `DEFAULT_ADMIN` role for itself, possibly leading to no administrators and loss of control of the contract |
There was a problem hiding this comment.
Should the Renounce function be stopped from removing the last DEFAULT_ADMIN_ROLE? Assets have / are implementing this style of function in their code
There was a problem hiding this comment.
Updates: revokeRole and renounceRole are now protected from removing the last DEFAULT_ADMIN_ROLE.
lfportal
force-pushed
the
TD-1396-immutable-signed-zone-v2-threat-model
branch
from
9f49e27 to
21fe3d0
Compare
lfportal
force-pushed
the
TD-1396-immutable-signed-zone-v2-threat-model
branch
from
21fe3d0 to
44ae6b2
Compare
Threat model for Immutable Signed Zone v2.