Rangers Protocol

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.

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.

Commit: - [`0248879b845ff1a9e790580655cf7daf784536d1`](https://github.com/rangersprotocolcode/go-rangers/tree/0248879b845ff1a9e790580655cf7daf784536d1) Files: - src/core/genesis_sub.go - src/core/genesis_block.go - src/core/genesis_block_dev.go - src/core/genesis_block_robin.go When iterating over the array `genesis.Group.Members`, the function `getSubGenesisValidators()` does not check if the length is equal. If the length of `genesis.Group.Members` is greater than the length of `genesis.Pks` or `genesis.VrfPKs`, an index-out-of-range may be raised, which causes the init flow of blockchain to fail. The same issue exists in the following functions. - `src/core/genesis_block.go#genGenesisBlock()` - `src/core/genesis_block_dev.go#genDevGenesisBlock()` - `src/core/genesis_block_robin.go#genRobinGenesisBlock()`

Commit: - [`0248879b845ff1a9e790580655cf7daf784536d1`](https://github.com/rangersprotocolcode/go-rangers/tree/0248879b845ff1a9e790580655cf7daf784536d1) Files: - src/core/groupchain.go The functions `LastGroup()` and `Count()` are used to read group info of the chain, which will be updated by multi-goroutine. However, the current implementation lacks appropriate synchronization mechanisms, leading to potential race conditions. Without proper synchronization, there is no guarantee that the values returned by `lastGroup` and `count` reflect the most recent and correct state of the blockchain, which could result in incorrect data being read and acted upon. ```go func (chain *groupChain) LastGroup() *types.Group { return chain.lastGroup } func (chain *groupChain) Count() uint64 { return chain.count } ```