MCC Staking - Smart Contract Audit Report

Summary

MCC Staking Audit Report Multi-Chain Capital is building a new contract so users can deploy staking contracts based on Multi-Chain Capital's existing template for depositing tokens and earning yield.

For this audit, we reviewed the project team's MCCStaking and MCCStakingPool contracts at commit 6055198a975c26953232a37707ffc8f5d3177786 on the team's Github repository.

We previously reviewed the project team's ERC-20 token here.

Audit Findings

Please ensure trust in the team prior to investing as they have some control in the ecosystem.
Date: January 28th, 2022.
Finding #1 - MCCStakingPool - Low
Description: When autocompounding harvested rewards on behalf of another specified user, the function intends to stake the specified user's reward tokens, but stakes the caller's tokens instead.
function harvestForUser(
    uint256 _rewardsIndex,
    address _userAddy,
    bool _autoCompound
) external returns (uint256) {
        
    _updatePool(_rewardsIndex);
    uint256 _tokensToUser = _harvestTokens(_rewardsIndex, _userAddy);

    if (
      _autoCompound &&address(_rewardsTokens[_rewardsIndex].token) == address(_stakedToken)
    ) {
      stakeTokens(_tokensToUser);
    }

    return _tokensToUser;
}
Risk/Impact: The caller may end up unintentionally staking their own tokens while attempting to use the autocompound feature.
Recommendation: The team should set the stakeTokens() function as internal as well as add an address parameter in place of "msg.sender". A new public function should be implemented that passes msg.sender and an integer amount to the stakeTokens() function for regular staking deposits. The _harvestForUser() function should then be pass the _userAddy parameter to the stakeTokens() function if the _autoCompound condition is true.
Finding #2 - MCCStakingPool - Low
Description: The owner can bypass the contract lock time condition in the MCCStaking contract by using the removeRewardsToken() function in the MCCStakingPool() function to withdraw reward tokens.
function removeRewardsToken(uint256 _rewardsIndex) public {
    require(
      msg.sender == poolCreator || msg.sender == tokenOwner,
      'caller must be the contract creator or owner to remove stakable tokens'
    );
    _rewardsTokens[_rewardsIndex].token.transfer(
      tokenOwner,
      _rewardsTokens[_rewardsIndex].curRewardsSupply
    );
    _rewardsTokens[_rewardsIndex] = _rewardsTokens[_rewardsTokens.length - 1];
    _rewardsTokens.pop();
}
Risk/Impact: By bypassing the contract lock time the owner can withdraw all reward tokens before the contract lock time has elapsed. This contradicts the MCCStaking contract's lock time conditional check in the removeTokenContract() function.
Recommendation: We recommend implementing a conditional check within the removeRewardsToken() function so the original reward tokens cannot be removed until after the contract lock time has elapsed. Alternatively, the team can update the current conditional check so that only the MCCStaking contract can access the removeRewardsToken() function.

Contracts Overview

MCCStakingPool Contract:
This contract allows users to stake specified tokens into the contract in order to earn rewards in the form of reward tokens determined by the project team.
On deposits, users are minted 1 $sMCC token for each token that they deposit.
Staked funds are locked for a specified period set on deployment; the lock time is reset each time the user performs an additional deposits.
Users can withdraw their staked tokens from the pool once the lock time has elapsed or the total reward supply has been depleted. An equivalent of $sMCC tokens are subsquently sent to the dead address.
On deposits and withdrawals, any pending rewards are calculated and transferred to the user unless the total reward supply has been depleted.
Reward amounts are calculated based on time staked, set reward per block rate, and total reward supply value.
The contract allows anyone to harvest rewards on behalf of any user at any time. If the caller chooses to autocompound the calculated rewards they are intended to be re-staked into the staking pool for the specified user. However the contract instead attempts to stake an equivalent amount of the callers own tokens.
The owner can withdraw any tokens from the contract at any time.
The owner can add or remove reward tokens to/from the contract at any time.
The owner should not add ERC-777 compliant tokens as staking tokens.
The owner can deposit additional reward tokens to the contract at any time.
The owner should not use fee-on-transfer tokens as reward tokens. If a fee-on-transfer token is used as a reward token, then the contract must be exempt from transfer fees.
The owner can transfer ownership to another address at any time.
As the contract is implemented with Solidity v0.8.x, it is protected from overflow/underflow attacks along with following the ERC-20 standard.
MCCStaking Contract:
Anyone can utilize this contract to deploy an MCCStakingPool contract with customized staking criteria; the user specifies the staking token address, reward token address, reward supply, reward rate, contract lock time, and staked token lock time.
The user can specify a lock time of 0, which will lock the rewards in contract forever.
Owners can withdraw the reward tokens from their own MCCStakingPool contract once the contract lock time has elapsed.
As the contract is implemented with Solidity v0.8.x, it is protected from overflow/underflow attacks.

External Threat Results

Vulnerability Category	Notes	Result
Arbitrary Storage Write	N/A	PASS
Arbitrary Jump	N/A	PASS
Centralization of Control	The owner can withdraw the reward tokens at any time.	WARNING
Delegate Call to Untrusted Contract	N/A	PASS
Dependence on Predictable Variables	N/A	PASS
Deprecated Opcodes	N/A	PASS
Ether Thief	N/A	PASS
Exceptions	N/A	PASS
External Calls	N/A	PASS
Flash Loans	N/A	PASS
Integer Over/Underflow	N/A	PASS
Logical Issues	The harvestForUser() function attempts to stake tokens for the incorrect user.	FAIL
Multiple Sends	N/A	PASS
Oracles	N/A	PASS
Suicide	N/A	PASS
State Change External Calls	N/A	PASS
Unchecked Retval	N/A	PASS
User Supplied Assertion	N/A	PASS
Critical Solidity Compiler	N/A	PASS
Overall Contract Safety		FAIL

MCCStakingPool Contract

BEP20 Token Graph

Multi-file Token

        
            ($) = payable function
 # = non-constant function

+ [Int] IERC20 
    - [Ext] totalSupply
    - [Ext] balanceOf
    - [Ext] transfer #
    - [Ext] allowance
    - [Ext] approve #
    - [Ext] transferFrom #

 + [Int] IERC20Metadata (IERC20)
    - [Ext] name
    - [Ext] symbol
    - [Ext] decimals

 +  Context 
    - [Int] _msgSender
    - [Int] _msgData

 +  ERC20 (Context, IERC20, IERC20Metadata)
    - [Pub] 
            
                #
    - [Pub] name
    - [Pub] symbol
    - [Pub] decimals
    - [Pub] totalSupply
    - [Pub] balanceOf
    - [Pub] transfer #
    - [Pub] allowance
    - [Pub] approve #
    - [Pub] transferFrom #
    - [Pub] increaseAllowance #
    - [Pub] decreaseAllowance #
    - [Int] _transfer #
    - [Int] _mint #
    - [Int] _burn #
    - [Int] _approve #
    - [Int] _beforeTokenTransfer #
    - [Int] _afterTokenTransfer #

 + [Lib] SafeMath 
    - [Int] tryAdd
    - [Int] trySub
    - [Int] tryMul
    - [Int] tryDiv
    - [Int] tryMod
    - [Int] add
    - [Int] sub
    - [Int] mul
    - [Int] div
    - [Int] mod
    - [Int] sub
    - [Int] div
    - [Int] mod

 +  MCCStakingPool (ERC20)
    - [Pub] #
       - modifiers: ERC20
    - [Ext] updateInitialRewardsSupply #
    - [Pub] addNewRewardsToken #
       - modifiers: onlyTokenOwner
    - [Ext] addToRewardsSupply #
       - modifiers: onlyTokenOwner
    - [Ext] stakedTokenAddress
    - [Ext] rewardsTokenAddresses
    - [Ext] removeAllRewardsTokens #
    - [Pub] removeRewardsToken #
    - [Pub] stakeTokens #
    - [Ext] unstakeTokens #
    - [Ext] harvestForUser #
    - [Pub] getLastStakableBlock
    - [Pub] calcHarvestTot
    - [Ext] setTokenOwner #
       - modifiers: onlyTokenOwner
    - [Prv] _updatePool #
    - [Prv] _harvestTokens #
    - [Prv] _updNumStaked #

MCCStaking Contract

BEP20 Token Graph

Multi-file Token

        
            ($) = payable function
 # = non-constant function
 + [Int] IERC20 
    - [Ext] totalSupply
    - [Ext] balanceOf
    - [Ext] transfer #
    - [Ext] allowance
    - [Ext] approve #
    - [Ext] transferFrom #

 + [Int] IERC20Metadata (IERC20)
    - [Ext] name
    - [Ext] symbol
    - [Ext] decimals

 +  Context 
    - [Int] _msgSender
    - [Int] _msgData

 +  ERC20 (Context, IERC20, IERC20Metadata)
    - [Pub] 
            
                #
    - [Pub] name
    - [Pub] symbol
    - [Pub] decimals
    - [Pub] totalSupply
    - [Pub] balanceOf
    - [Pub] transfer #
    - [Pub] allowance
    - [Pub] approve #
    - [Pub] transferFrom #
    - [Pub] increaseAllowance #
    - [Pub] decreaseAllowance #
    - [Int] _transfer #
    - [Int] _mint #
    - [Int] _burn #
    - [Int] _approve #
    - [Int] _beforeTokenTransfer #
    - [Int] _afterTokenTransfer #

 + [Lib] SafeMath 
    - [Int] tryAdd
    - [Int] trySub
    - [Int] tryMul
    - [Int] tryDiv
    - [Int] tryMod
    - [Int] add
    - [Int] sub
    - [Int] mul
    - [Int] div
    - [Int] mod
    - [Int] sub
    - [Int] div
    - [Int] mod

 +  MCCStakingPool (ERC20)
    - [Pub] #
       - modifiers: ERC20
    - [Ext] updateInitialRewardsSupply #
    - [Pub] addNewRewardsToken #
       - modifiers: onlyTokenOwner
    - [Ext] addToRewardsSupply #
       - modifiers: onlyTokenOwner
    - [Ext] stakedTokenAddress
    - [Ext] rewardsTokenAddresses
    - [Ext] removeAllRewardsTokens #
    - [Pub] removeRewardsToken #
    - [Pub] stakeTokens #
    - [Ext] unstakeTokens #
    - [Ext] harvestForUser #
    - [Pub] getLastStakableBlock
    - [Pub] calcHarvestTot
    - [Ext] setTokenOwner #
       - modifiers: onlyTokenOwner
    - [Prv] _updatePool #
    - [Prv] _harvestTokens #
    - [Prv] _updNumStaked #

 +  MCCStaking 
    - [Ext] getAllFarmingContracts
    - [Ext] getTokensForStaking
    - [Ext] createNewTokenContract ($)
    - [Ext] removeTokenContract #

Go Home

Solidity Finance

MCC Staking - Smart Contract Audit Report

Summary

Audit Findings

Finding #1 - MCCStakingPool - Low

Finding #2 - MCCStakingPool - Low