CyberConnect - Audit

The function `_requiresExpectedSigner()` is used to check if `expectedSigner` is the correct signer, which will be called by the function `preProcess()` from the contract `PermissionMw`. ```solidity function _requiresExpectedSigner( bytes32 digest, address expectedSigner, uint8 v, bytes32 r, bytes32 s, uint256 deadline ) internal view { require(deadline >= block.timestamp, "DEADLINE_EXCEEDED"); require( uint256(s) <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0, "INVALID_SIGNATURE_S_VAULE" ); address recoveredAddress = ecrecover(digest, v, r, s); require(recoveredAddress == expectedSigner, "INVALID_SIGNATURE"); } ``` If `ecrecover()` fails to recover an address, a zero address will be returned, which is not checked in `_requiresExpectedSigner()`. If the role `NameRegistry` assign a zero address to the signer through the function `setMwData()`, an invalid signature could bypass the permission check of the function `preProcess()` in the middleware `PermissionMw`. ```solidity //file src/middlewares/mocaid/PermissionMw.sol function setMwData(bytes calldata data) external override onlyNameRegistry { signer = abi.decode(data, (address)); } function preProcess( DataTypes.RegisterNameParams calldata params, bytes calldata data ) external payable override onlyNameRegistry { DataTypes.EIP712Signature memory sig; (sig.v, sig.r, sig.s, sig.deadline) = abi.decode( data, (uint8, bytes32, bytes32, uint256) ); _requiresExpectedSigner( _hashTypedDataV4( keccak256( abi.encode( Constants._REGISTER_TYPEHASH, keccak256(bytes(params.name)), params.parentNode, params.to, nonces[params.to]++, sig.deadline ) ) ), signer, sig.v, sig.r, sig.s, sig.deadline ); } ``` References: - https://docs.soliditylang.org/en/v0.8.9/units-and-global-variables.html?highlight=ecrecover#mathematical-and-cryptographic-functions

In the contract `StableFeeMiddleware`, the function `usdOracle.getRoundData()` returns the token's Oracle price. According to the documentation, it must validate the returned **timestamps**, **round ID**, and **price** to ensure the price is valid. ```solidity function _getPriceAt(uint80 roundId) internal view returns (int256) { // prettier-ignore ( /* uint80 roundID */, int price, /*uint startedAt*/, /*uint timeStamp*/, /*uint80 answeredInRound*/ ) = usdOracle.getRoundData(roundId); return price; } ``` Moreover, The function **_getPrice()** is using `latestRoundData()` to get the token's price by Chainlink, but there are no validations that the data is not stale. Reference: - https://docs.chain.link/data-feeds/historical-data#getrounddata-return-values

The value for moca xp can be updated via `setMocaXP` with a limitation of `_MAX_XP` by the operator. ```solidity=374 function setMocaXP(uint256 tokenId, uint256 xp) external { require(xp <= _MAX_XP, "XP_TOO_BIG"); DataTypes.MetadataPair[] memory pairs = new DataTypes.MetadataPair[](1); pairs[0] = DataTypes.MetadataPair(_MOCA_XP_KEY, xp.toString()); batchSetGatedMetadatas(tokenId, pairs); emit MocaXPSet(tokenId, xp); } ``` However, since `batchSetGatedMetadatas` is external function, the operator can invoke `batchSetGatedMetadatas` directly to bypass the validation on moca xp value.

When register cyber ID, there is a pre-register process when the middleware of the contract is not address(0) ```solidity=177 function register( string calldata cid, address to, bytes32 secret, uint8 durationYear, bytes calldata middlewareData ) external payable { ... uint256 cost; if (middleware != address(0)) { cost = ICyberIdMiddleware(middleware).preRegister{ value: msg.value }( DataTypes.RegisterCyberIdParams( msg.sender, cid, to, durationYear ), middlewareData ); } _register(cid, to, durationYear, cost); } ``` During the preregister process in stable fee middleware, it will query the fee and calculate the cost: ```solidity=83 function preRegister( DataTypes.RegisterCyberIdParams calldata params, bytes calldata data ) external payable override returns (uint256) { uint80 roundId = abi.decode(data, (uint80)); uint256 cost = getPriceWeiAt(params.cid, roundId, params.durationYear); _chargeAndRefundOverPayment(cost, params.msgSender); return cost; } ``` However, the `roundID` is based on the `middlewareData`, which is user's input. As a result, the user has the chance to select the price for the registration.

The following function are missing access control in `StableFeeMiddleware`: - `preRegister` - `preRenew` - `preBid`

In contract `TrustOnlyMiddleware`, the `_owner` can be updated by multiple parties 1. via `setMwData`(by NameRegistry) 2. via `transferOwnership`(by current owner).

The function `register()` is intended to register a new cid and ultimately calls the function `preRegister()` from `middleware`, which will charge the required native tokens. ```solidity function register( string calldata cid, address to, bytes32 secret, uint8 durationYear, bytes calldata middlewareData ) external payable { //... uint256 cost; if (middleware != address(0)) { cost = ICyberIdMiddleware(middleware).preRegister{ value: msg.value }( DataTypes.RegisterCyberIdParams( msg.sender, cid, to, durationYear ), middlewareData ); } _register(cid, to, durationYear, cost); } ``` However, if the middleware is zero address, the function `preBid()` won't be called and no cost will be calculated. As a result, the receiving token `msg.value` will be locked in the contract permanently. Moreover, the same thing will happen in other functions `renew()` and `bid()`.