Quadrata Passport - Audit (Addendum)

In the contract `QuadGovernance` the role `DEFAULT_ADMIN_ROLE` is the admin of `GOVERNANCE_ROLE`. In the contract `QuadGovernance` the role `GOVERNANCE_ROLE` is the admin of `PAUSER_ROLE`, `ISSUER_ROLE` and `READER_ROLE`, and also has authority over the functions below. - function setTreasury() - function setPassportContractAddress() - function updateGovernanceInPassport() - function acceptGovernanceInPassport() - function setEligibleAttribute() - function setEligibleAttributeByDID() - function setPreapprovals() - function setAttributePriceFixed() - function setBusinessAttributePriceFixed() - function setRevSplitIssuer() - function addIssuer() - function deleteIssuer() - function setIssuerStatus() - function setIssuerAttributePermission() - function _authorizeUpgrade(): upgrade contract. In the contract `QuadGovernance` the role `OPERATOR_ROLE` has authority over the function below. - function setTokenURI() In the contract `QuadPassport` the role `GOVERNANCE_ROLE` of `QuadGovernance` contract has authority over the functions below. - function withdraw(): withdraw received fees to an issuer's treasury. - function setQuadReader(): set `reader` contract. - function migrateAttributes(): migrate attributes from `_attributes` to `_attributesv2`. - function _authorizeUpgrade(): upgrade contract. In the contract `QuadPassport` the role `PAUSER_ROLE` of `QuadGovernance` contract has authority over the functions below. - function pause() - function unpause() In the contract `QuadPassport` the role `ISSUER_ROLE` of `QuadGovernance` contract has authority to sign passport configuration data, which can be used to update passports. In the contract `QuadPassport` the role `READER_ROLE` of `QuadGovernance` contract has authority over the functions below. - function attribute(): get one attribute information about a passport. - function attributes(): get all attribute information about a passport. In the contract `QuadReader` the role `GOVERNANCE_ROLE` of `QuadGovernance` contract has authority over the functions below. - function withdraw() - function _authorizeUpgrade(): upgrade contract. `QuadPassport`, `QuadSoulbound`, `QuadReader` and `QuadGovernance` are upgradeable contracts, the owner can upgrade the contract without the community's commitment. Any compromise to the privileged account may allow the hacker to take advantage of this authority.

The `burnPassports()` and `burnPassportsIssuer()` functions are used to burn users' passports with all attributes. These functions iterate through the `eligibleAttributesArray` array, which contains eligible attributes. However, there may be other attributes that are true in the `_eligibleAttributesByDID` mapping that are not included in the `eligibleAttributesArray`. As a result, the `burnPassports()` and `burnPassportsIssuer()` functions may not burn all passports.

The function `_setAttributesVerify()` does not check whether a `signedMsg` has been used before. This means that a `signedMsg` can be reused, potentially allowing a user to recover a passport after it has been removed.

The current migration process involves removing the previous data right after the migration. However, this practice is not recommended as it can lead to the loss of important data in case of unexpected errors or issues.

The `_issuerTreasury` map stores the issuer's treasury address. The `addIssuer()` function sets this address for the new issuer, but the `deleteIssuer()` function doesn't remove the deleted issuer's treasury address, which causes inconsistency in state.

The deleted issuer will be saved in the `_deletedIssuers` array. The `addIssuer()` function doesn't check whether the new issuer to be added is in the `_deletedIssuers` array. Therefore, the rejoined issuer may exist in both the `_issuers` and `_deletedIssuers` arrays at the same time.

```solidity function attribute( address _account, bytes32 _attribute ) public view override returns (Attribute memory) { require(msg.sender == address(reader) || IAccessControlUpgradeable(address(governance)).hasRole(READER_ROLE, _msgSender()), "INVALID_READER"); address[] memory issuers = governance.getIssuers(); for (uint256 i = 0; i < issuers.length; i++) { bytes32 attrKey = attributeKey(_account, _attribute, issuers[i]); Attribute memory attr = _attributesv2[attrKey]; if (attr.epoch != uint256(0)) { attr.issuer = issuers[i]; return attr; } } return Attribute({value: bytes32(0), epoch: uint256(0), issuer: address(0)}); } ``` The `attribute()` function in the contract is designed to return a single `Attribute` struct for the inputted `_account` and `_attribute`. However, if multiple issuers sign the same `_attribute` for the `_account`, there could be multiple `Attribute` structs, potentially leading to data inconsistency issues and confusion in the passport verification process.

```solidity contract QuadGovernanceStoreV2 is QuadGovernanceStore { address[] internal _deletedIssuers; mapping(address => bool) internal _preapprovals; } ``` ```solidity contract QuadGovernance is IQuadGovernance, AccessControlUpgradeable, UUPSUpgradeable, QuadGovernanceStoreV2 { ``` In the current design, a new contract `QuadGovernanceStoreV2` is created by inheriting from an existing contract `QuadGovernanceStore`, which is then inherited by the implementation contract `QuadGovernance`. This approach is not recommended because if a developer upgrades `QuadGovernance` in the future without looking at the previous version, it is possible that new state variables that conflict with existing storage slots will be introduced to the new `QuadGovernance` contract. Contract `QuadPassportStoreV2` shares the same issue.