---
# System prepended metadata

title: Final Security Audit of Narfex MasterChef contract by profiterole.group

---

![](https://i.imgur.com/7hfXLAh.png)


# Final Security Audit of Narfex MasterChef contract by profiterole.group

## Conclusion

This audit was made by Profiterole Group https://profiterole.group/
Auditor: Vladimir Smelov.
Date: 2023-04-15

Based on the evaluation, the Client's smart contracts are considered secure.

Our team conducted a code functionality analysis, manual examination, test cases and automated tests using static analyzers. All issues identified during the automated analysis were manually inspected, and significant vulnerabilities are highlighted in the Audit Summary section.

The Client has acknowledged and fixed or accepted all the issues.

## Scope

### Pre Audit Scope

repo: https://github.com/narfex/masterchef
commit: `916751ad7f8679c54ee5de2b5c87d72958bd7e7e`

- contracts/MasterChef.sol (md5 `efe5d889e727a7e55f4898d740ee61fa`)

### Final Audit Scope

repo: https://github.com/narfex/masterchef
commit: `b6544d0f7501208b4c841e09f031b392bddda5f0`

- contracts/MasterChef.sol (md5 `f49b20a87dec35690d9ceb6ca7ee3286`)
- contracts/utils/EmergencyState.sol (md5 `0613e96e5beed801115b9f8039659aa3`)

### Final Deployed Contract

MasterChef on Ethereum - https://etherscan.io/address/0x877Ec50E446B74058CFA38935CCe83408B71D746#code

## Methodology

1. Blind audit. Understand the structure of the code without reading any docs.
2. Questions to developers.
3. Execute static analyzers.
4. Identify problems with:
- backdoors;
- bugs;
- mathematical errors;
- potential fund leakage;
- potential contract lockup;
- argument and event validation;
- others.
5. Draft report formation and discussing results with the client.
6. Final report, ensuring all security vulnerabilities have been fixed.

## Contract desrciption

This contract represents a decentralized farming platform focused on the NRFX token, which allows users to deposit various types of liquidity provider (LP) tokens into the platform in order to earn rewards. The platform supports multiple pools, each with its own allocation of rewards, represented by the NRFX token. Users can deposit their LP tokens into a specific pool, and the contract will distribute rewards to users based on their share of the total deposited tokens in the pool. The contract also includes functionality for referral rewards, early withdrawal fees, and other mechanisms to optimize reward distribution and encourage user participation.

The core functionality of the contract includes depositing LP tokens, withdrawing LP tokens, and harvesting rewards. Users can deposit their LP tokens into a specific pool, optionally specifying a referral address to receive a referral reward. The contract also allows for the withdrawal of LP tokens and harvesting of earned rewards. Additionally, the contract includes safeguards against various attack vectors, such as Reentrancy and Emergency situations, and ensures the proper accounting and distribution of rewards. The contract also has functionality to recover accidentally sent tokens, maintaining the overall safety and security of the platform.

## Results

#### MAJOR-1.

At
- MasterChef.sol:219 https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L219

```solidity=219
        return !isEarlyWithdraw;
```

should it be just "isEarlyWithdraw" not "!isEarlyWithdraw" otherwise the result will be opposite.

##### Status.
FIXED - the function was removed.


______

#### MAJOR-2.

At
- MasterChef.sol:226-229 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L226-L229
- MasterChef.sol:215-220 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L215-L220
- MasterChef.sol:204-209 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L204-L209
- MasterChef.sol:185-198 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L185-L198
- MasterChef.sol:291 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L291
- MasterChef.sol:269 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L269


```solidity=226
    function getUserPoolSize(address _pairAddress, address _user) public view returns (uint) {
        uint256 _pid = poolId[_pairAddress];
        return userInfo[_pid][_user].amount;
    }
```
```solidity=215
    function getIsEarlyWithdraw(address _pairAddress, address _user) internal view returns (bool) {
        uint256 _pid = poolId[_pairAddress];
        UserInfo storage user = userInfo[_pid][_user];
        bool isEarlyWithdraw = block.timestamp - user.depositTimestamp < earlyHarvestCommissionInterval;
        return !isEarlyWithdraw;
    }
```
```solidity=204
    function getIsUserCanHarvest(address _pairAddress, address _user) internal view returns (bool) {
        uint256 _pid = poolId[_pairAddress];
        UserInfo storage user = userInfo[_pid][_user];
        bool isEarlyHarvest = block.timestamp - user.harvestTimestamp < harvestInterval;
        return !isEarlyHarvest;
    }
```
```solidity=185
    function getUserReward(address _pairAddress, address _user) public view returns (uint256) {
        uint256 _pid = poolId[_pairAddress];
        PoolInfo storage pool = poolInfo[_pid];
        UserInfo storage user = userInfo[_pid][_user];
        uint256 accRewardPerShare = pool.accRewardPerShare;
        uint256 lpSupply = pool.pairToken.balanceOf(address(this));
        if (block.number > pool.lastRewardBlock && lpSupply != 0) {
            uint256 blocks = block.number - pool.lastRewardBlock;
            uint256 totalReward = lastBalance * blocks - k * blocks**2 / 2;
            uint256 reward = totalReward * pool.allocPoint / totalAllocPoint;
            accRewardPerShare += reward * 1e12 / lpSupply;
        }
        return user.amount * accRewardPerShare / 1e12 - user.withdrawnReward + user.storedReward;
    }
```
```solidity=291
    function getPoolUserData(address _pairAddress, address _user) public view returns (
```
```solidity=269
        uint256 _pid = poolId[_pairAddress];
```

if there is no poolId for not-exist _pairAddress, then _pid=0, and the function will return amount of the user 0 pool (which has different LP token), consider adding _poolExists check. You can use it as a modifier like onlyExistPool(_pairAddress) and use it on every method with _pairAddress argument

##### Status.
FIXED - `onlyExistPool` modifier is placed on every public function with `_pairAddress` argument.


______

#### MAJOR-3.

At
- MasterChef.sol:409 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L409
- MasterChef.sol:430 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L430
- MasterChef.sol:443 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L443


```solidity=409
    function withdraw(address _pairAddress, uint256 _amount) public {
```

```solidity=430
    function emergencyWithdraw(address _pairAddress) public {
```

```solidity=443
    function harvest(address _pairAddress) public {
```


nonReentrant modifier is forgotten.

##### Status.
FIXED - `nonReentrant` modifier is placed on every public function changing state, preventing reentry attacks.

______

#### MAJOR-4.

There could be unfair rewards distribution in case if some used left his reward locked for a long time and during this time other users have harvested all available rewards. In this case long-lock user will receive no reward which is unfair.


##### Status.
FIXED - `endBlock` was introduced to guarantee that rewards on pool with the current `rewardsPerBlock` will be fairly distributed till `endBlock`

______

#### WARNING-1.

At
- MasterChef.sol:66 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L66

```solidity=66
    uint256 public lastBalance;
```

why we need it if we can use narfex.balanceOf(address(this))

##### Status.
FIXED - the attribute was removed.
There was another `lastRewardTokenBalance` added to account new rewards.


______

#### WARNING-2.

At
- MasterChef.sol:157 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L157

```solidity=157
        if (_withUpdate) {
```

At
- MasterChef.sol:173 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L173

```solidity=173
        if (_withUpdate) {
```

should it be required?

##### Status.
FIXED - `_massUpdatePools` is always called.


______

#### WARNING-3.

At
- MasterChef.sol:353 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L353
- MasterChef.sol:190 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L190

```solidity=353
        uint256 lpSupply = pool.pairToken.balanceOf(address(this));
```
```solidity=190
        uint256 lpSupply = pool.pairToken.balanceOf(address(this));
```

im not sure but what will happen if someone just transfer LP to the balance of this contract (not with deposit but just with erc20 transfer), will it ruin the calculations and lead to mistakes in reward sharing?

##### Status.
FIXED - `pool.totalDeposited` is used instead.
Moreover, LP tokens occasionaly sent to the contract may be recovered via `recoverERC20` (except of deposited).


______

#### WARNING-4.

At
- MasterChef.sol:436 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L436

```solidity=436
        user.amount = 0;
```

better to reset storage before external call

##### Status.
FIXED - 
1) `nonReentrant` modifier is used.
2) storage reset placed before external call.

______

#### LOW-1.

At
- MasterChef.sol:98 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L98

```solidity=98
    /// @notice Returns the soil fertility
```

what is "soil fertility"

##### Status.
FIXED - rephrased


______

#### LOW-2.

At
- MasterChef.sol:110 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L110
- MasterChef.sol:421 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L421
- MasterChef.sol:434 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L434


```solidity=110
        rewardToken.safeTransfer(address(msg.sender), amount);
```

```solidity=421
            pool.pairToken.safeTransfer(address(msg.sender), _amount);
```

```solidity=434
        pool.pairToken.safeTransfer(address(msg.sender), user.amount);
```
msg.sender is already an address no conversions needed

##### Status.
ACKNOWLEDGED


______

#### LOW-3.

At
- MasterChef.sol:193 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L193


```solidity=193
            uint256 totalReward = lastBalance * blocks - k * blocks**2 / 2;
```

unclear math. Consider the usage of some constant-speed rewards distribution.

##### Status.
FIXED - constant-speed rewards distribution is used.


______

#### LOW-4.

At
- MasterChef.sol:194 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L194


```solidity=194
            uint256 reward = totalReward * pool.allocPoint / totalAllocPoint;
```

it should check that totalAllocPoint!=0

##### Status.
ACKNOWLEDGED - totalAllocPoint may never ne zero because the owner will add pools with positive alloc points right after the deploy.


______

#### LOW-5.

At
- MasterChef.sol:204 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L204

```solidity=204
    function getIsUserCanHarvest(address _pairAddress, address _user) internal view returns (bool) {
```

prefix with _

##### Status.
FIXED - declared public


______

#### LOW-6.

At
- MasterChef.sol:215 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L215

```solidity=215
    function getIsEarlyWithdraw(address _pairAddress, address _user) internal view returns (bool) {
```

prefix with _

##### Status.
FIXED - removed


______

#### LOW-7.

At
- MasterChef.sol:226 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L226

```solidity=226
    function getUserPoolSize(address _pairAddress, address _user) public view returns (uint) {
```

better to rename to getUserPoolAmount

##### Status.
ACKNOWLEDGED


______

#### LOW-8.

At
- MasterChef.sol:462 - https://github.com/narfex/masterchef/blob/916751ad7f8679c54ee5de2b5c87d72958bd7e7e/contracts/MasterChef.sol#L462

```solidity=462
    function rewardTransfer(UserInfo storage user, uint256 _amount, bool isWithdraw, uint256 _pid) internal {
```

prefix with _

##### Status.
FIXED - renamed


______

#### LOW-9.

At
- MasterChef.sol:757-762 - https://github.com/narfex/masterchef/blob/b6544d0f7501208b4c841e09f031b392bddda5f0/contracts/MasterChef.sol#L757

```solidity=757
/// @notice Sets the early harvest commission
/// @param percents Early harvest commission in percents denominated by 10000 (1000 for default 10%)
function setEarlyHarvestCommission(uint percents) external onlyOwner nonReentrant {
    earlyHarvestCommission = percents;
    emit EarlyHarvestCommissionSet(percents);
}
```

- MasterChef.sol:737-740 - https://github.com/narfex/masterchef/blob/NEWNEW/contracts/MasterChef.sol#L737

```solidity=737
function setEarlyHarvestCommissionInterval(uint interval) external onlyOwner nonReentrant {
    earlyHarvestCommissionInterval = interval;
    emit EarlyHarvestCommissionIntervalSet(interval);
}
```

global attributes `earlyHarvestCommission`, `earlyHarvestCommissionInterval` are not used, consider removal

##### Status.
ACKNOWLEDGED - it's not a functional problem


