Draft

bitsCrunch Protocol Contracts - bitsCrunch


Prepared by:

Halborn Logo

HALBORN

Last Updated 03/07/2024

Date of Engagement by: February 5th, 2024 - February 16th, 2024

Summary

100% of all REPORTED Findings have been addressed

All findings

6

Critical

0

High

0

Medium

1

Low

2

Informational

3


1. INTRODUCTION

The bitsCrunch Protocol Smart Contracts are a set of Solidity contracts that exist on the Polygon Blockchain. The contracts enable a decentralized network for the users to query data. Node Operators then provide queries to users. Users pay for queries with the selected stable coins.

\client engaged Halborn to conduct a security assessment on their smart contracts beginning on 2024-02-05 and ending on 2024-02-16. The security assessment was scoped to the smart contracts provided in the bitscrunch-protocol/smartcontracts GitHub repository. Commit hashes and further details can be found in the Scope section of this report.

2. ASSESSMENT SUMMARY

Halborn was provided 2 weeks for the engagement and assigned a full-time security engineer to review the security of the smart contracts in scope. The security team consists of a blockchain and smart contract security experts with advanced penetration testing and smart contract hacking skills, and deep knowledge of multiple blockchain protocols.

The purpose of the assessment is to:

    • Identify potential security issues within the smart contracts.

    • Ensure that smart contract functionality operates as intended.

In summary, Halborn identified some security risks that were partially addressed by the bitsCrunch team. The main one was the following:

    • Calculate the amountOutMinimum parameter off-chain and pass it to the swapStableToBCUT() function of the Payments contract instead of using estimatedAmountOutcalculated in the function.

3. TEST APPROACH & METHODOLOGY

Halborn performed a combination of manual and automated security testing to balance efficiency, timeliness, practicality, and accuracy in regard to the scope of this assessment. While manual testing is recommended to uncover flaws in logic, process, and implementation; automated testing techniques help enhance coverage of the code and can quickly identify items that do not follow the security best practices. The following phases and associated tools were used during the assessment:

    • Research into architecture and purpose.

    • Smart contract manual code review and walkthrough.

    • Graphing out functionality and contract logic/connectivity/functions (solgraph).

    • Manual assessment of use and safety for the critical Solidity variables and functions in scope to identify any arithmetic related vulnerability classes.

    • Manual testing by custom scripts.

    • Static Analysis of security for scoped contract, and imported functions (Slither).

    • Testnet deployment (Foundry, Brownie).

4. SCOPE

Code repositories:

bitsCrunch Protocol Contracts

Repository: bitscrunch-protocol/smartcontracts

Commit ID: 54b65bc2433a1c1a63e610d3e6a9164e4d1cb382

Smart contracts in scope:

    • contracts/Bitscrunch/*

    • contracts/Contributor/*

    • contracts/Epochs/*

    • contracts/GovernanceController/*

    • contracts/Operator/*

    • contracts/Token/*

    • contracts/Utils/*

4.1 Out-of-scope

    • External libraries and financial-related attacks.

    • New features/implementations after/with the remediation commit IDs.

5. RISK METHODOLOGY

Every vulnerability and issue observed by Halborn is ranked based on two sets of Metrics and a Severity Coefficient. This system is inspired by the industry standard Common Vulnerability Scoring System.
The two Metric sets are: Exploitability and Impact. Exploitability captures the ease and technical means by which vulnerabilities can be exploited and Impact describes the consequences of a successful exploit.
The Severity Coefficients is designed to further refine the accuracy of the ranking with two factors: Reversibility and Scope. These capture the impact of the vulnerability on the environment as well as the number of users and smart contracts affected.
The final score is a value between 0-10 rounded up to 1 decimal place and 10 corresponding to the highest security risk. This provides an objective and accurate rating of the severity of security vulnerabilities in smart contracts.
The system is designed to assist in identifying and prioritizing vulnerabilities based on their level of risk to address the most critical issues in a timely manner.

5.1 EXPLOITABILITY

Attack Origin (AO):
Captures whether the attack requires compromising a specific account.
Attack Cost (AC):
Captures the cost of exploiting the vulnerability incurred by the attacker relative to sending a single transaction on the relevant blockchain. Includes but is not limited to financial and computational cost.
Attack Complexity (AX):
Describes the conditions beyond the attacker’s control that must exist in order to exploit the vulnerability. Includes but is not limited to macro situation, available third-party liquidity and regulatory challenges.
Metrics:
EXPLOITABILITY METRIC (mem_e)METRIC VALUENUMERICAL VALUE
Attack Origin (AO)Arbitrary (AO:A)
Specific (AO:S)
1
0.2
Attack Cost (AC)Low (AC:L)
Medium (AC:M)
High (AC:H)
1
0.67
0.33
Attack Complexity (AX)Low (AX:L)
Medium (AX:M)
High (AX:H)
1
0.67
0.33
Exploitability EE is calculated using the following formula:

E=meE = \prod m_e

5.2 IMPACT

Confidentiality (C):
Measures the impact to the confidentiality of the information resources managed by the contract due to a successfully exploited vulnerability. Confidentiality refers to limiting access to authorized users only.
Integrity (I):
Measures the impact to integrity of a successfully exploited vulnerability. Integrity refers to the trustworthiness and veracity of data stored and/or processed on-chain. Integrity impact directly affecting Deposit or Yield records is excluded.
Availability (A):
Measures the impact to the availability of the impacted component resulting from a successfully exploited vulnerability. This metric refers to smart contract features and functionality, not state. Availability impact directly affecting Deposit or Yield is excluded.
Deposit (D):
Measures the impact to the deposits made to the contract by either users or owners.
Yield (Y):
Measures the impact to the yield generated by the contract for either users or owners.
Metrics:
IMPACT METRIC (mIm_I)METRIC VALUENUMERICAL VALUE
Confidentiality (C)None (I:N)
Low (I:L)
Medium (I:M)
High (I:H)
Critical (I:C)
0
0.25
0.5
0.75
1
Integrity (I)None (I:N)
Low (I:L)
Medium (I:M)
High (I:H)
Critical (I:C)
0
0.25
0.5
0.75
1
Availability (A)None (A:N)
Low (A:L)
Medium (A:M)
High (A:H)
Critical (A:C)
0
0.25
0.5
0.75
1
Deposit (D)None (D:N)
Low (D:L)
Medium (D:M)
High (D:H)
Critical (D:C)
0
0.25
0.5
0.75
1
Yield (Y)None (Y:N)
Low (Y:L)
Medium (Y:M)
High (Y:H)
Critical (Y:C)
0
0.25
0.5
0.75
1
Impact II is calculated using the following formula:

I=max(mI)+mImax(mI)4I = max(m_I) + \frac{\sum{m_I} - max(m_I)}{4}

5.3 SEVERITY COEFFICIENT

Reversibility (R):
Describes the share of the exploited vulnerability effects that can be reversed. For upgradeable contracts, assume the contract private key is available.
Scope (S):
Captures whether a vulnerability in one vulnerable contract impacts resources in other contracts.
Metrics:
SEVERITY COEFFICIENT (CC)COEFFICIENT VALUENUMERICAL VALUE
Reversibility (rr)None (R:N)
Partial (R:P)
Full (R:F)
1
0.5
0.25
Scope (ss)Changed (S:C)
Unchanged (S:U)
1.25
1
Severity Coefficient CC is obtained by the following product:

C=rsC = rs

The Vulnerability Severity Score SS is obtained by:

S=min(10,EIC10)S = min(10, EIC * 10)

The score is rounded up to 1 decimal places.
SeverityScore Value Range
Critical9 - 10
High7 - 8.9
Medium4.5 - 6.9
Low2 - 4.4
Informational0 - 1.9

6. SCOPE

Files and Repository
(a) Repository: smartcontracts
(b) Assessed Commit ID: 54b65bc
(c) Items in scope:
  • contracts/Epochs/EpochManager.sol
  • contracts/Epochs/IEpochManager.sol
  • contracts/Epochs/EpochManagerStorage.sol
↓ Expand ↓
Out-of-Scope: External libraries and financial-related attacks., New features/implementations after/with the remediation commit IDs.
Remediation Commit ID:
  • 749b7f3
Out-of-Scope: New features/implementations after the remediation commit IDs.

7. Assessment Summary & Findings Overview

Critical

0

High

0

Medium

1

Low

2

Informational

3

Security analysisRisk levelRemediation Date
HAL-01 - INAPPROPRIATE SLIPPAGE CONTROLMediumRisk Accepted - 03/06/2024
HAL-06 - IMPROPER OPERATOR STAKING AMOUNT CALCULATIONLowSolved - 02/17/2024
HAL-04 - USING TRANSFER INSTEAD OF SAFETRANSFERLowRisk Accepted - 02/16/2024
HAL-05 - ARBITRARY SENDER USED IN PULLTOKENSInformationalAcknowledged - 02/16/2024
HAL-02 - GAS INEFFICIENT ERROR HANDLINGInformationalAcknowledged - 02/16/2024
HAL-03 - GAS INEFFICENT INCREMENTATIONInformationalAcknowledged - 02/16/2024

8. Findings & Tech Details

8.1 (HAL-01) INAPPROPRIATE SLIPPAGE CONTROL

// Medium

Description

The swapStableToBCUT() function of the Payments contract uses the _swapExactInputSingle() function to exchange tokens.

Exchanging tokens requires the user to specify the amountOutMinimum parameter. If the swap results in less, the transaction reverts. This parameter is by the _getAmountOut() function, which calculates the value based on the price just before the transaction.

Exchanging large amounts of tokens at once may encourage malicious users to perform a sandwich attack, causing loss to the protocol.

Note that the effectiveness of sandwich attacks largely depends on the available liquidity in the swap or liquidity pools or the number of tokens exchanged at once. Attackers usually only carry out such an attack if it is profitable for them. If they can predict the time of the swaps, they can increase the price seconds before the swap transaction is executed.

Code Location

Input

contracts/Bitscrunch/Payments/Payments.sol

    function swapStableToBCUT(uint256 _epoch, IERC20Upgradeable _stableCoin) external onlyManager nonReentrant {
        
        _checkAndRevert(coins_[_stableCoin].active, "P_1");
        _checkAndRevert(_epoch <= epochManager_.currentEpoch(), "P_2");

        uint256 toSwap = epochBilledBalance_[_epoch][_stableCoin];

        _checkAndRevert(toSwap > 0, "P_3");

        uint256 toTreasury = _getShareAmount(toSwap, shares_.treasury, percentage_);
        uint256 tokensLeft = toSwap - toTreasury;

        uint256 toBcut = _getShareAmount(tokensLeft, shares_.bcut, percentage_);
        uint256 stableCoinLeft = tokensLeft - toBcut;

        uint256 estimatedAmountOut = _getAmountOut(address(_stableCoin),address(bcutToken_),uint128(toBcut));
        uint256 bcutReceived = _swapExactInputSingle(address(_stableCoin), address(bcutToken_), toBcut, estimatedAmountOut);

        balances_.treasury[address(_stableCoin)] += toTreasury;
        
        balances_.coinBalance[_epoch][address(bcutToken_)] += bcutReceived;
        balances_.coinBalance[_epoch][address(_stableCoin)] += stableCoinLeft;
BVSS
Recommendation

Recommendation

It is recommended to allow the manager to configure the average price time frame for the calculation of the amountOutMinimum value. The timeframe should be set so that malicious users can only raise and maintain the manipulated price more expensively than their expected profit from the sandwich attack.

Alternatively, the amountOutMinimum parameter can be calculated off-chain and pass it to the swapStableToBCUT() function. This approach is more gas efficient and allows the manual control of the minimum amount of tokens that the protocol expects to receive.


Remediation Plan

RISK ACCEPTED: The bitsCrunch team accepted the risk of the identified issue regarding the swap function's operation within irregular intervals and the calculation of amountOutMinimum.

8.2 (HAL-06) IMPROPER OPERATOR STAKING AMOUNT CALCULATION

// Low

Description

Operators cannot stake more than the minimumStakeAmount_ defined in the Staking contract. However, it was identified that the stakeBCUT(), unstakeBCUT() and _isStaked() functions assume that this fixed staking amount does not change. If the Owner increases the stake amount using the setMinimumStakeAmount() function, then the Operators might be able to stake more than the amount defined in the variable, but they will not be able to withdraw the full amount. If the Owner lowers the stake amount, Operators can only withdraw part of their staked funds.

Code Location

Input

contracts/Operator/Staking/Staking.sol

  function stakeBCUT() external {

    address staker = _msgSender();

    _checkAndRevert(operator_.isOperatorActive(staker), "Staking: Operator is not active");
    _checkAndRevert(!_isStaked(staker), "Staking: Operator Already staked");

    Operator storage currentOperator = operatorInfo_[staker];
    uint256 currentEpoch = epochManager_.currentEpoch();

    if(currentOperator.lastStakedEpoch == 0){
        operatorInfo_[staker] = Operator(minimumStakeAmount_, currentEpoch, 0, lockPeriod_);
    }
    else {

        currentOperator.balance += minimumStakeAmount_;
        currentOperator.lastStakedEpoch = currentEpoch;
        currentOperator.lockPeriod = lockPeriod_;

        operatorInfo_[staker] = currentOperator;
    }

    currentStaking_ += minimumStakeAmount_;
    TokenUtils.pullTokens(bcutToken_,staker,minimumStakeAmount_);

    emit StakerBalance(staker, operatorInfo_[staker].balance, currentEpoch, minimumStakeAmount_, "add");

  }

Input

contracts/Operator/Staking/Staking.sol

  function unstakeBCUT() external {

    address staker = _msgSender();
    _checkAndRevert(_isStaked(staker), "Staking: Operator Not Staked");

    Operator storage currentOperator = operatorInfo_[staker];
    uint256 epochPassed = epochManager_.epochsSince(currentOperator.lastStakedEpoch);

    _checkAndRevert(epochPassed >= currentOperator.lockPeriod, "Staking: Tokens Locked");

    currentOperator.balance -= minimumStakeAmount_;
    operatorInfo_[staker] = currentOperator;

    currentStaking_ -= minimumStakeAmount_;

    TokenUtils.pushTokens(bcutToken_,staker,minimumStakeAmount_);
    emit StakerBalance(staker, operatorInfo_[staker].balance, epochManager_.currentEpoch(), minimumStakeAmount_, "remove");
  }

Input

contracts/Operator/Staking/Staking.sol

    function _isStaked(address _operator) internal view returns(bool) {
      return operatorInfo_[_operator].balance >= minimumStakeAmount_;
    }
BVSS
Recommendation

Recommendation

It is recommended to only increase the staked balance up to minimumStakeAmount_ during staking and withdraw the full staked balance during withdrawals.

Remediation Plan

SOLVED: The bitsCrunch team solved the issue in commit 749b7f3.

Remediation Hash
749b7f347324f2765d4f01005ceb84f51b3d7ca6

8.3 (HAL-04) USING TRANSFER INSTEAD OF SAFETRANSFER

// Low

Description

It was identified that the pullTokens() and pushTokens() functions in the TokenUtils contract use the IERC20Upgradeable interface to interact with the _tokenparameter to transfer currencies between the contract and the target wallet. The IERC20Upgradeable interface expects the transfer functions to have a return value on success. It is important to note that the transfer functions of some tokens (e.g., USDT, BNB on the Ethereum network) do not return any values, so these tokens are incompatible with the current version of the contracts using the functions from the TokenUtils contract.

Code Location

Input

contracts/Utils/TokenUtils.sol

    function pullTokens(
        IERC20Upgradeable _token,
        address _from,
        uint256 _amount
    ) internal {
        if (_amount > 0) {
            bool success = _token.transferFrom(_from, address(this), _amount);
            if(!success){
                revert TokenTransferError("Tokens: Couldn't transfer");
            }
        }
    }

Input

contracts/Utils/TokenUtils.sol

    function pushTokens(
        IERC20Upgradeable _token,
        address _to,
        uint256 _amount
    ) internal {
        if (_amount > 0) {
            bool success = _token.transfer(_to, _amount);
            if(!success){
                revert TokenTransferError("Tokens: Couldn't transfer");
            }
        }
    }

BVSS
Recommendation

Recommendation

It is recommended to use OpenZeppelin's SafeERC20 wrapper with the IERC20Upgradeableinterface to make the contracts compatible with currencies that return no value.

Remediation Plan

RISK ACCEPTED: The bitsCrunch team made a business decision to accept the risk of this finding and not alter the contracts until they plan to support such tokens. The Protocol intended to be used with USDC as the stable coin.

8.4 (HAL-05) ARBITRARY SENDER USED IN PULLTOKENS

// Informational

Description

It was identified that the receiveRewards() functions in the ContributorRewardManager and OperatorRewardManagercontracts can pull tokens from arbitrary addresses. These functions are used by the Managers to transfer rewards the contracts for the source addresses that previously approved token transfers.

Code Location

Input

contracts/Contributor/RewardManager/RewardManager.sol

   function receiveRewards(uint256 _epoch, address _from, uint256 _amount) external onlyManager nonReentrant {
                
        _checkAndRevert(_epoch < epochManager_.currentEpoch(), "Invalid Epoch Passed");
        _checkAndRevert(epochRewards_[_epoch].settledRewards == 0, "Settlement Already Started");

        TokenUtils.pullTokens(bcutToken_,_from,_amount);
        epochRewards_[_epoch].totalRewards += _amount;

        emit ReceivedRewards(_epoch,_amount,epochRewards_[_epoch].totalRewards);
    }

Input

contracts/Operator/RewardManager/RewardManager.sol

   function receiveRewards(uint256 _epoch, address _from, uint256 _amount) external onlyManager nonReentrant {
                
        _checkAndRevert(_epoch < epochManager_.currentEpoch(), "Invalid Epoch Passed");
        _checkAndRevert(epochRewards_[_epoch].settledRewards == 0, "Settlement Already Started");

        TokenUtils.pullTokens(bcutToken_,_from,_amount);
        epochRewards_[_epoch].totalRewards += _amount;

        emit ReceivedRewards(_epoch,_amount,epochRewards_[_epoch].totalRewards);
    }
BVSS
Recommendation

Recommendation

It is recommended to only pull tokens from the msg.sender or from a predefined address.

Remediation Plan

ACKNOWLEDGED: The bitsCrunch team made a business decision to acknowledge this finding and not alter the contracts. The manager of the contract will be a timelock controller, and the manager's wallet will be managed by AWS KMS. This reward-receiving model is the intended design in that the token-transferring user will first approve the amount of tokens, and the manager will initiate the function to receive rewards for the contract.

8.5 (HAL-02) GAS INEFFICIENT ERROR HANDLING

// Informational

Description

It was identified that long revert strings are used in the several contracts. Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. If the revert string uses strings to provide additional information about failures, but they are rather expensive, especially when it comes to deploying cost, and it is difficult to use dynamic information in them.

Long revert strings were identified in the following contracts:

  • contracts/Contributor/RewardManager/RewardManager.sol

  • contracts/Contributor/Staking/ContributorStaking.sol

  • contracts/Epochs/EpochManager.sol

  • contracts/GovernanceController/Governance/Controller.sol

  • contracts/Operator/Operator.sol

  • contracts/Operator/RewardManager/RewardManager.sol

  • contracts/Operator/Staking/Staking.sol

  • contracts/Token/BCUTToken.sol

Score
Recommendation

Recommendation

Consider implementing custom errors instead of reverting strings or using shorter error messages.

Remediation Plan

ACKNOWLEDGED: The bitsCrunch team made a business decision to acknowledge this finding and not alter the contracts.

8.6 (HAL-03) GAS INEFFICENT INCREMENTATION

// Informational

Description

It was identified that postfix (e.g. i++) operators were used in several contracts to increment loop iterator variables. It is known that, in loops, using prefix operators (e.g. ++i) costs less gas per iteration than postfix operators.

Postfix operators were used in the following contracts:

  • contracts/Bitscrunch/Customer/CustomerStaking.sol

  • contracts/Bitscrunch/Payments/CustomerPayout.sol

  • contracts/Bitscrunch/Payments/OperatorPayout.sol

  • contracts/Contributor/RewardManager/RewardManager.sol

  • contracts/Operator/RewardManager/RewardManager.sol

Score
Recommendation

Recommendation

Consider using prefix operation instead of postfix to increment the values of the uint loop iterator variables.

Remediation Plan

ACKNOWLEDGED: The bitsCrunch team made a business decision to acknowledge this finding and not alter the contracts.

9. Automated Testing

STATIC ANALYSIS REPORT

Description

Halborn used automated testing techniques to enhance the coverage of certain areas of the smart contracts in scope. Among the tools used was Slither, a Solidity static analysis framework. After Halborn verified the smart contracts in the repository and was able to compile them correctly into their abis and binary format, Slither was run against the contracts. This tool can statically verify mathematical relationships between Solidity variables to detect invalid or inconsistent usage of the contracts' APIs across the entire code-base. The security team assessed all findings identified by the Slither software, however, findings with severity Information and Optimization are not included in the below results for the sake of report readability.

Results

  • contracts/Bitscrunch/*

    Bitscrunch1.pngBitscrunch2.png
  • contracts/Contributor/*

    Contributor1.pngContributor2.png
  • contracts/Epochs/*

    Epochs.png
  • contracts/GovernanceController/*

    Slither did not identify any vulnerabilities.


  • contracts/Operator/*

    Operator.png
  • contracts/Token/*

    Token.png
  • contracts/Utils/*

    Utils.png

Results summary

The findings obtained as a result of the Slither scan were reviewed.

  • The reentrancy, dangerous strict equality and local variable never initialized vulnerabilities were determined false-positives.

  • The arbitrary from in transferFrom vulnerability was added to the report.

Halborn strongly recommends conducting a follow-up assessment of the project either within six months or immediately following any material changes to the codebase, whichever comes first. This approach is crucial for maintaining the project’s integrity and addressing potential vulnerabilities introduced by code modifications.

© Halborn 2024. All rights reserved.