---
# System prepended metadata

title: Security Audit of `Narfex-masterchef` contracts.

---

# Security Audit of `Narfex-masterchef` contracts. 

## Conclusion

This audit was made by 
Auditor: Vladimir Smelov <vladimirfol@gmail.com>.
Date: 2023-03-20

TODO

## Scope

TODO

## Methodology

1. Blind audit. Comprehend the structure of the code without reading any docs.
2. Ask questions to developers.
3. Run static analyzers.
4. Find problems with:
- backdoors;
- bugs;
- math;
- potential leaking of funds;
- potential locking of the contract;
- validate arguments and events;
- others.

## Result

#### MAJOR-1.

At
- MasterChef.sol:219

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

should it be just "isEarlyWithdraw" not "!isEarlyWithdraw"

##### Status.
NEW


______

#### MAJOR-2.

At
- MasterChef.sol:226-229,215-220,204-209,185-198,291,269

```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.
NEW


______

#### MAJOR-3.

At
- MasterChef.sol:409

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

nonReentrant

##### Status.
NEW


______

#### MAJOR-4.

At
- MasterChef.sol:430

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

nonReentrant

##### Status.
NEW


______

#### MAJOR-5.

At
- MasterChef.sol:443

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

nonReentrant

##### Status.
NEW


______

#### WARNING-1.

At
- MasterChef.sol:66

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

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

##### Status.
NEW


______

#### WARNING-2.

At
- MasterChef.sol:157

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

should it be required?

##### Status.
NEW


______

#### WARNING-3.

At
- MasterChef.sol:173

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

should it be required? otherwise miscalculation

##### Status.
NEW


______

#### WARNING-4.

At
- MasterChef.sol:353,190

```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.
NEW


______

#### WARNING-5.

At
- MasterChef.sol:436

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

better to reset storage before external call

##### Status.
NEW


______

#### LOW-1.

At
- MasterChef.sol:98

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

what is "soil fertility"

##### Status.
NEW


______

#### LOW-2.

At
- MasterChef.sol:110

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

msg.sender is already an address no conversions needed

##### Status.
NEW


______

#### LOW-3.

At
- MasterChef.sol:193

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

hmmm

##### Status.
NEW


______

#### LOW-4.

At
- MasterChef.sol:194

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

to be honest, it should check that totalAllocPoint!=0

##### Status.
NEW


______

#### LOW-5.

At
- MasterChef.sol:204

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

prefix with _

##### Status.
NEW


______

#### LOW-6.

At
- MasterChef.sol:215

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

prefix with _

##### Status.
NEW


______

#### LOW-7.

At
- MasterChef.sol:226

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

better to rename to getUserPoolAmount

##### Status.
NEW


______

#### LOW-8.

At
- MasterChef.sol:421

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

msg.sender is already an address no conversions needed

##### Status.
NEW


______

#### LOW-9.

At
- MasterChef.sol:434

```solidity=434
        pool.pairToken.safeTransfer(address(msg.sender), user.amount);
```

msg.sender is already an address no conversions needed

##### Status.
NEW


______

#### LOW-10.

At
- MasterChef.sol:462

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

prefix with _

##### Status.
NEW


