Rangers Protocol security record

Commit: - [`0248879b845ff1a9e790580655cf7daf784536d1`](https://github.com/rangersprotocolcode/go-rangers/tree/0248879b845ff1a9e790580655cf7daf784536d1) Files: - src/core/blockchain.go The `TopBlock`, `Height` and `TotalQN` methods return `latestBlock` or the respective fields from it. Such a property can be written at any time by the go-routines that handles the blockchain functioning. Any access to it should be synchronized to solve any possible race condition. In this case, the pointed getters directly read such data without any synchronization mechanism, which can cause a fatal error and the node to crash. The `remove` method removes a block from the current chain state. When the `chain.latestBlock` field is updated, no thread synchronization mechanism is put in place. Since the variable can be concurrently accessed by several go-routines, the code may crash with a fatal error due to the unhandled race condition. The `buildCache` method loads in the `topBlocks` cache the latest blocks according to the `size` parameter. However, when `latestBlock` is accessed, there is not any mechanism to synchronize with potential concurrent go-routines that may have already started at the node boot.

Commit: - [`0248879b845ff1a9e790580655cf7daf784536d1`](https://github.com/rangersprotocolcode/go-rangers/tree/0248879b845ff1a9e790580655cf7daf784536d1) Files: - src/core/sync_processor.go The `syncing` property in `syncProcessor` is used by the `trySync` function to avoid executing its logic in the case a previous execution is still pending. Such behavior should properly handle the race condition on `syncing`. While the following block ```go=216 p.lock.Lock("trySync") defer p.lock.Unlock("trySync") ``` protects the variable when reading and writing respectively in L#218 and L#235, there is not any guarantee that a write will not happen during the check in L#211. In fact, such condition, in the developer intent, is implicitly assumed as possible given the need to include a second check of the condition in L#218.

The logic ensures the cross-chain transaction atomicity is not implemented in the contract. The cross-chain functions emit messages to 3rd party application. The scope of the audit treats the 3rd party application entities as black boxes and assumes their functional correctness. However, in the real world, 3rd parties can be compromised, which may lead to lost or stolen assets. Suppose the cross-chain transaction atomicity is not guaranteed properly. In that case, the user deposits tokens into the source chain, but not be able to redeem the correct token amount from the target chain.

The `IsInPoolfts` function checks whether a token contract address (addr) exists in the `g_pool_fts` array, which acts as a pool or whitelist of token addresses. However, the contract exhibits problematic behavior when `_fromcontract` is not in `g_pool_fts`. Specifically: ### Minting of Non-Whitelisted Tokens: When `_fromcontract` is not in `g_pool_fts`, the contract can still call `IERC20(_fromcontract).mint(address(this), am)`. This is counterintuitive to the purpose of a whitelist, which is typically used to control and limit the minting or interaction to approved token contracts. Allowing minting of tokens outside the whitelist undermines the trustworthiness and control of the contract over its operations. ### No Explicit Minting Whitelist: The `IsInPoolfts` function seems to check for inclusion in the pool (g_pool_fts), but its logic does not explicitly enforce a prohibition against minting non-whitelisted tokens. This ambiguity makes it difficult for users and auditors to determine which tokens the contract is allowed to mint, leading to potential exploitation. ```solidity function IsInPoolfts(address addr) private view returns(bool) { for(uint256 i = 0; i < g_pool_fts.length; i++) { if(g_pool_fts[i] == addr){ return true; } } return false; } function AddPoolFts(address[] memory addr) external{ require(msg.sender == g_Setter ,'p1'); for(uint256 i=0;i<addr.length; i++){ if(IsInPoolfts(addr[i]) == false){ g_pool_fts.push(addr[i]); } } } ... if(IsInPoolfts(_fromcontract) == false){ IERC20(_fromcontract).mint(address(this),am); } ```

The specified input parameters lack validation. Specifically, a check should be implemented to confirm whether `groupid` already exists in `g_CheckAddrs`.

The function `DoCrossErc20_CM()` charges user's ERC20 tokens and send message to bridge. When the conditions are not met, the native fee tokens will not be sent back to the user, as the transaction will still succeed due to the `return` statement. ```solidity g_FeeAddr.transfer(msg.value); ... if(IERC20(_fromcontract).balanceOf(_fromaddr) >= amount && IERC20(_fromcontract).allowance(_fromaddr,address(this)) >= amount) { ... } emit event_CrossErc20_Failed_CM(msg.value,_fromcontract,_tocontract,_toChainId,_fromaddr,_toaddr,am); return; ``` The function `DoCrossErc20_CM2()` shares the same issue.

Commit: - [`0248879b845ff1a9e790580655cf7daf784536d1`](https://github.com/rangersprotocolcode/go-rangers/tree/0248879b845ff1a9e790580655cf7daf784536d1) Files: - src/core/vmexecutor_sub.go - src/core/subcontracts/economy/Economy.sol The function `calcSubCoinReward` is utilized to calculate rewards and send coins to callers/proposers/validators via the following code. ```go vmInstance := vm.NewEVMWithNFT(vmCtx, executor.accountdb, executor.accountdb) caller := vm.AccountRef(vmCtx.Origin) code, done := executor.generateCode(proposals, validators, member, header) if done { return } codeBytes := common.FromHex(code) _, _, _, err := vmInstance.Call(caller, common.EconomyContract, codeBytes, vmCtx.GasLimit, big.NewInt(0)) if err != nil { rewardLog.Errorf("calcSubCoinReward error: %s. code: %s", err.Error(), code) } ``` The aforementioned code snippet will call the function `calc()` from a smart contract `Economy` defined in `src/core/subcontracts/economy/Economy.sol`. ```solidity contract Economy{ //... function calc(address payable proposal, address payable[] calldata allProposal, address payable[] calldata validators) external { require(msg.sender==address(0x1111111111111111111111111111111111111111),'calc caller wrong'); uint256 reward = calcPerBlock(block.number); proposal.transfer(reward.mul(proposalRate).div(100)); uint256 otherProposal = reward.mul(otherProposalRate).div(100); uint256 totalStake = 0; for(uint256 i=0;i<allProposal.length;i++){ totalStake = totalStake.add(allProposal[i].stakenum); } for(uint256 i=0;i<allProposal.length;i++){ //allProposal[i].balance = allProposal[i].balance+otherProposal * allProposal[i].stakenum/totalStake allProposal[i].transfer(otherProposal.mul(allProposal[i].stakenum).div(totalStake)); } uint256 validatorReward = reward.mul(uint256(100).sub(otherProposalRate).sub(proposalRate)).div(100); uint256 totalValidatorStake = 0; for(uint256 i=0;i<validators.length;i++){ totalValidatorStake = totalValidatorStake.add(validators[i].stakenum); } for(uint256 i=0;i<validators.length;i++){ //validators[i].balance = validators[i].balance+validatorReward * validators[i].stakenum/totalValidatorStake validators[i].transfer(validatorReward.mul(validators[i].stakenum).div(totalValidatorStake)); } } function calcPerBlock(uint256 height) /*internal*/public view returns(uint256){ //... } function getEpoch(uint256 height) internal view returns(uint256) { return height.div(epoch); } } ``` In one case, if the developer intends to use the contract `Economy` directly. Since `Economy` does not declare any payable function and defines the receive function, no native tokens can be transferred to the contract `Economy`. The contract `Economy` will fail to send coins to corresponding accounts in `calc()`. ```solidity receive() external payable { } ``` In the other case, if the developer intends to use `EcomonyProxy` as the proxy contract utilizing the transparent proxy pattern. Since the function does not initialize the state variables `epoch` in an `initialize` function, the epoch will be a zero value, see [Writing Upgradeable Contracts](https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable). A division-by-zero error will be raised when the function `calc` is called and no reward coins will be sent to related accounts. No reward coins will be sent to miners/proposals/validators in both cases. **NOTE** The smart contract files are out of the scope of this audit commitment.

Commit: - [`0248879b845ff1a9e790580655cf7daf784536d1`](https://github.com/rangersprotocolcode/go-rangers/tree/0248879b845ff1a9e790580655cf7daf784536d1) Files: - src/core/blockchain_verify.go It was observed that the `verifyBlock` function is capable of parsing blocks that contain 0 transactions. While this behavior may be intentional, the presence of several consecutive empty blocks could potentially indicate irregularities in transaction volume or miner behavior. This raises concerns about the expected functionality and the robustness of the system under atypical conditions.

Commit: - [`0248879b845ff1a9e790580655cf7daf784536d1`](https://github.com/rangersprotocolcode/go-rangers/tree/0248879b845ff1a9e790580655cf7daf784536d1) Files: - src/core/blockchain.go The `QueryBlock` method locks the global lock to perform a query to fetch a block. However, all the other similar functions `QueryBlockByHash` and `QueryBlockHeaderByHeight` perform a similar task assuming the underlying accessed caches and databases are already thread-safe, so they do not block any additional lock.

Commit: - [`0248879b845ff1a9e790580655cf7daf784536d1`](https://github.com/rangersprotocolcode/go-rangers/tree/0248879b845ff1a9e790580655cf7daf784536d1) Files: - src/core/blockchain_verify.go Transaction and Receipts in EVM-like chains are stored in the form of Merkle trees or Merkle tries in order to quickly verify their integrity and to index them. However, code at the pointed location computes a Tree by simply concatenating hashes in a buffer and hashing them.