Nayms

The `NaymsOwnershipFacet.transferOwnership()` function can accidentally revoke the role when the `_newOwner` is the same as the current owner. The function first calls `LibACL._assignRole()` and then calls `LibACL._unassignRole()`. If the `_newOwner` is the same as the current owner, the first call `LibACL._assignRole()` will override the same thing, and the role will be immediately revoked by the call of `LibACL._unassignRole()`.

We recommend adding validations to the Nayms system for several reasons. First, the system has multiple layers of function calls between facets, libraries, and storage. While libraries are often designed to be more generalized, facet functions may have more specific responsibilities within a smaller scope. In such cases, it is recommended to add validations to ensure the feature is only doing what the facet function is intended to support and nothing more. Second, Solidity provides empty default values for all variable types, which can create risks when getting struct data from a mapping. It will always provide empty struct data regardless of whether the key-value pair has been set. Finally, adding validations can help prevent accidental wrong inputs that could mutate the system. 1. `EntityFacet.enableEntityTokenization()`: should validate that only an "entity" can tokenize with this function (e.g., checking `s.existingEntities[_entityId] == true`). The current implementation allows tokenizing any object with this function which violates the function naming. 1. `LibEntity._createEntity()`: the fact that the `_entityAdmin` exists and that the `_entityID` does not already exist is not checked (e.g., checking `s.existingEntities[_entityId] == true`). Also, if given empty inputs, it can assign value to an empty key for the `s.existingEntities[]`, `s.entities[]`, `s.objectParent[]`, and `s.roles[]` mappings. The pattern is dangerous. For instance, it can get an unexpected entity with `s.entities[LibObject._getParent(_policyId)]` when the `_policyId` does not exist. 1. `LibEntity._updateEntity()`: the function should validate that `_entityId` is not empty. Otherwise, it can assign value to an empty key for the `s.entities[]` mapping. The pattern is dangerous. For instance, it can get an unexpected entity with `s.entities[LibObject._getParent(_policyId)]` when the `_policyId` does not exist. 1. `LibEntity._updateAllowSimplePolicy()`: the function should validate that `_entityId` is not empty. 1. `LibEntity._startTokenSale()`: the function should validate that `s.existingEntities[_entityId] == true` and that the `entity` has enabled tokenization (e.g., by calling `LibObject._isObjectTokenizable()`). 1. `LibEntity._createSimplePolicy()`: 1. the function should validate that `_entityId` and `_policyId` are not empty bytes. 2. the function should validate the fields of the `_stakeholders`. First, the `_stakeholders.roles.length` should be the same as the `_stakeholders.entityIds.length` (or the length of the `signatures`). Second, the `_stakeholders.entityIds[i]` should be the same as the `_entityId`. Third, depending on the business requirement, all the roles of the `_stakeholders.roles[i]` should probably be the same. Without the validations, it can set the policy role to another entity, and different stakeholders might have different roles. 3. if the length of the parameter `_stakeholders` is too high, it can lead to an out-of-gas error in the for-loop. 1. `LibEntity.validateEntity()`: there is no check making sure that the `utilizedCapacity` of the entity is less than or equal to its `maxCapacity`. 1. `LibEntity. _validateSimplePolicyCreation()`: there is no min duration for the policy, no non-zero check for the `claimsPaid` and the `premiumPaids`, no min/max check for the `sponsorCommissionBasisPoints` and no check that the boolean attribute `canceled` is `false`. 1. `UserFacet.setEntity()`: the `_entityId` should be linked to an existing entity. Also, the function should check whether the user has already been attached to an entity. Otherwise, it can have unexpected outcomes, such as the user not being able to cancel orders for the previous entity anymore. 1. `LibSimplePolicy._payPremium()`: the function can add validation checking that the entity for `_payerEntityId` exists and the policy for `_policyId` exists. Though unlikely, the dummy `simplePolicy` returned as the default value from the mapping might pass through the function and have an unexpected effect. 1. `LibACL._assignRole()`: the `_objectId`, `_contextId`, and `_roleId` inputs should not be empty strings. 1. `LibACL._updateRoleAssigner()`: the `_role` and the `_assignerGroup` should not be empty. Specifically the `_role` input. It can set a value to the `s.canAssign[]` mapping for the empty key. 1. `LibACL._updateRoleGroup()`: the `_role` and `_group` inputs should not be empty strings. Otherwise, it can set the values for empty keys in the `s.groups` mapping. The pattern is dangerous because it can potentially break the `LibACL._canAssign()` function. The `assignerGroup` with the current implementation can be empty bytes when `_roleId` does not exist in the `s.canAssign[]` mapping. The `_isInGroup()` check might pass and falsely return true. 1. `LibAdmin._addSupportedExternalToken()`: the `_tokenAddress` input should not be empty. 1. `LibAdmin`: there is no non-zero address check for the parameter `_tokenAddress` in the functions `_a

The state variable `supportedExternalTokens` is used to record the external tokens supported by the protocol. A new token can be added with the function `_addSupportedExternalToken()`. As the whole list is browsed with a for loop, it can be possible that, once a given size is reached, the function reverts with an out-of-gas error. In this case, it will not be possible anymore to add new supported external tokens.