MARBLEX security record

In the contract `MBXToken` the role `_owner` has authority over the functions shown in the diagram below. Additionally, the `_owner` has authority over the following functions: - `transferOwnership()` Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and - set the `trustedForwarder` to one that contains malicious logic in updating the `_msgSender()` within the `MBXToken` contract, possibly allowing for the stealing of funds from users; - freeze any account to prevent user interaction with their funds; - add accounts to the `MINTER_ROLE` allowing these accounts to mint any amount of tokens to any address; - add accounts to the `PAUSER_ROLE` allowing these accounts to pause any functionality in the contract that includes the modifier `whenNotPaused`, including - all transfer functions - all approval functions - all burning functions - remove accounts from the `MINTER_ROLE` or `PAUSER_ROLE` preventing the intended use of these roles; - withdraw any ERC20 or ERC721 token sent to the contract; - transfer the following privileged roles to one account through `acceptOwnership()`, giving all access control to one malicious authority - `_owner` - `DEFAULT_ADMIN_ROLE` - `MINTER_ROLE` - `PAUSER_ROLE`  --- In the contract `MBXToken` the role `DEFAULT_ADMIN_ROLE` has authority over the following functions: - `grantRole()` - `revokeRole()` - `revokeMinter()` - `revokePauser()` Any compromise to the `DEFAULT_ADMIN_ROLE` account may allow the hacker to take advantage of this authority and give access to the `MINTER_ROLE` or `PAUSER_ROLE`, allowing any amount of tokens to be minted to any account or pausing the contract to prevent interaction. Additionally, the attacker may use the authority to remove a legitimate account's ability to pause the contract during malicious takeover.

All of the `MBXToken` are sent to the contract deployer when deploying the contract, where the deployer specifies the amount to be minted. This could be a centralization risk as the deployer can distribute tokens without obtaining the consensus of the community. Any compromise to the deployer account that holds undistributed tokens may allow the attacker to steal and sell tokens on the market, resulting in severe damage to the project.

### `MBXToken.sol` Function `getNonce()` returns `super.nonces(from)`, where `from` is a user-provided input. The mapping `nonces` has a compiler-generated getter function which returns the same output. ### `MultiSigWallet.sol` Function `getTransaction()` returns the same information that is returned from directly referencing the compiler-generated getter function for the `transactions` array.

A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects. *This finding is considered minor because the reentrancy only causes out-of-order events.* ### External call(s) ```solidity=101 (bool success, ) = transaction.to.call{value: transaction.value}(transaction.data); ``` ### Events emitted after the call(s) ```solidity=104 emit ExecuteTransaction(msg.sender, _txIndex); ``` --- ### External call(s) ```solidity=39 (bool success, bytes memory returndata) = req.to.call{gas: req.gas, value: req.value}( abi.encodePacked(req.data, req.from) ); ``` ### Events emitted after the call(s) ```solidity=61 emit MetaTransactionExecuted(req.from, req.to, req.data); ```

Hooks `_beforeTokenApprove()` and `_beforeTokenTransfer()` are both used to add the same checks that the `from` address is not frozen (modifier `notFrozen`), and that the contract is not paused (modifier `whenNotPaused`). There are some functions in which both modifiers are called more than once on the same input. - Inherited function `transferFrom()` uses both hooks because `_spendAllowance()` calls function `_approve()` which is overridden to include `_beforeTokenApprove()`, and because internal `_transfer()` is called which includes `_beforeTokenTransfer()`; - Function `permit()` is overridden to include modifiers `notFrozen` and `whenNotPaused`, and its inherited logic calls internal function `_approve()` - Inherited function `burnFrom()` uses both hooks because `_spendAllowance()` calls function `_approve()` which is overridden to include `_beforeTokenApprove()`, and because internal `_burn()` includes `_beforeTokenTransfer()`; --- Internal function `_beforeTokenApprove()` also includes a reference to `super._beforeTokenTransfer()` in the body of the function. This references the inherited logic of the `_beforeTokenTransfer()` function, which also includes a check that the contract is not `paused`. The reference to `super._beforeTokenTransfer()` is unnecessary in the body of the `_beforeTokenApprove()` function. In the override of `_beforeTokenTransfer()` within the `MBXToken` contract, there is also a reference to `super._beforeTokenTransfer()`. Since this logic includes a check that the contract is not paused, it is not necessary to include the modifier `whenNotPaused` in the override of the `_beforeTokenTransfer()` function.

The return values of the `transfer()` and `transferFrom()` calls in the smart contract are not checked. Some ERC-20 tokens' transfer functions return no values, while others return a bool value, they should be handled with care. If a function returns `false` instead of reverting upon failure, an unchecked failed transfer could be mistakenly considered successful in the contract. ```solidity=135 IERC20(token).transfer(to, amount); ```