Smart Contract Audit

EGSwap
October 12, 2023

Summary

Vidma team has conducted a smart contract audit for the given codebase.

The contracts are in good condition. Based on the fixes provided by the Ammbr team and on the quality and security of the codebase provided, Vidma team can give a score of 95 to the audited smart contracts.

During the auditing process, the Vidma team has found a couple of informational issues, 7 issues with a low level of severity, 1 issue with a medium level of severity, and 3 issues with a critical level of severity.

Severity of the issue
Total found
Resolved
Unresolved
Critical
3 issues
3 issues
0 issues
High
3 issues
3 issues
0 issues
Medium
3 issues
3 issues
0 issues
Low
3 issues
3 issues
0 issues
Informational
3 issues
3 issues
0 issues
Low
3 issues
3 issues
0 issues

The contracts are in good condition. Based on the fixes provided by the Ammbr team and on the quality and security of the codebase provided, Vidma team can give a score of 95 to the audited smart contracts.

Based on the given findings, risk level, performance, and code style, Vidma team can grant the following overall score:

Please mind that this audit does not certify the definite reliability and security level of the contract. This document describes all vulnerabilities, typos, performance issues, and security issues found by Vidma auditing team. If the code is under development, we recommend run one more audit once the code is finalized.

Summary

Vidma is pleased to present this audit report outlining our assessment of code, smart contracts, and other important audit insights and suggestions for management, developers, and users.

The audited scope included contracts that allow trading one token for another and staking contract for farms. For Exchange liquidity pools, a fixed 0.25% trading fee is applied, which is broken down as follows: 0.17% returned to liquidity providers 0.08% kept by EG.

The auditor team found issues, most of which did not significantly impact the contract’s ability to operate. After the first review most issues are resolved or not relevant after logic changing & removing functionality, some of them were partly fixed but some points are missed. After second review the rest of issues are fixed excluding 1 issue (informational).

The changes are reflected in this version of the report accordingly.

During the audit process, the Vidma team found several issues, including those with critical severity. A detailed summary and the current state are displayed in the table below.

Severity of the issue Issue severity
Critical
High
Medium
Low
Informational
Total
Severity of the issue Issue severity Total found Resolved Invalid Unresolved
Critical 0 issues 0 issues 0 issues 0 issues
High 3 issues 2 issues 1 issue 0 issues
Medium 0 issues 0 issues 0 issues 0 issues
Low 10 issues 9 issues 1 issue 0 issues
Informational 3 issues 2 issues 0 issues 1 issue
Total 16 issues 13 issues 2 issues 1 issue

After evaluating the findings in this report and the final state after fixes, the Vidma auditors can state that the contracts are fully operational and secure. Under the given circumstances, we set the following risk level:

High Confidence

To set the codebase quality mark, our auditors are evaluating the initial commit given for the scope of the audit and the last commit with the fixes. This approach helps us adequately and sequentially evaluate the quality of the code. Code style, optimization of the contracts, the number of issues, and risk level of the issues are all taken into consideration. The Vidma team has developed a transparent evaluation codebase quality system presented below.

Severity of the issue
Issue severity
Total found
Resolved
Critical
1
10
High
0.8
7
Medium
0.5
5
Low
0.2
0.5
Informational
0
0.1
Please note that the points are deducted out of 100 for each and every issue on the list of findings (according to the current status of the issue). Issues marked as “not valid” are not subject to point deduction.
Codebase quality:
96.70

Evaluating the initial commit and the last commit with the fixes, Vidma audit team set the following codebase quality mark.

Score
Based on the overall result of the audit and the state of the final reviewed commit, the Vidma audit team grants the following score:

96.70

In addition to manual check and static analysis, the auditing team has conducted a number of integrated autotests to ensure the given codebase has an adequate performance and security level. The test results and coverage can be found in the accompanying section of this audit report.

Please be aware that this audit does not certify the definitive reliability and security level of the contract. This document describes all vulnerabilities, typos, performance issues, and security issues found by the Vidma audit team.
If the code is still under development, we highly recommend running one more audit once the code is finalized.

Scope of work

EGSwap represents a transformative step forward in the decentralized finance (DeFi) landscape.

Within the scope of this audit, two independent auditors thoroughly investigated the given codebase and analyzed the overall security and performance of the smart contracts.

The scope of work for the given audit consists of the following contracts:

  • EGSwapFactory;
  • EGSwapRouter;
  • Masterchef.

The source code was taken from the following source:

https://github.com/EG-Ecosystem/EGSwap-SmartContracts

Initial commit submitted for the audit:

bcd34de12385c6d4824d0ade95e3d409b925c251

Last commit reviewed by the auditing team:

e8544d6dab3dfa25f808ab758768396671db400f

As a reference to the contracts logic, business concept, and the expected behavior of the codebase, the EGSwap team has provided the following documentation:

https://docs.egswap.exchange/

Workflow of the auditing process

Vidma audit team uses the most sophisticated and contemporary methods and well-developed techniques to ensure contracts are free of vulnerabilities and security risks. The overall workflow consists of the following phases:

Phase 1: The research phase

Research

After the Audit kick-off, our security team conducts research on the contract’s logic and expected behavior of the audited contract.

Documentation reading

Vidma auditors do a deep dive into your tech documentation with the aim of discovering all the behavior patterns of your codebase and analyzing the potential audit and testing scenarios.

The outcome

At this point, the Vidma auditors are ready to kick off the process. We set the auditing strategies and methods and are prepared to conduct the first audit part.

Phase 2: Manual part of the audit

Manual check

During the manual phase of the audit, the Vidma team manually looks through the code in order to find any security issues, typos, or discrepancies with the logic of the contract. The initial commit as stated in the agreement is taken into consideration.

Static analysis check

Static analysis tools are used to find any other vulnerabilities in smart contracts that were missed after a manual check.

The outcome

An interim report with the list of issues.

Phase 3: Testing part of the audit

Integration tests

Within the testing part, Vidma auditors run integration tests using the Truffle or Hardhat testing framework. The test coverage and the test results are inserted in the accompanying section of this audit report.

The outcome

Second interim report with the list of new issues found during the testing part of the audit process.

Structure and organization of the findings

For simplicity in reviewing the findings in this report, Vidma auditors classify  the findings in accordance with the severity level of the issues. (from most critical to least critical).

All issues are marked as “Resolved” or “Unresolved”, depending on if they have been fixed by project team or not. The issues with “Not Relevant” status are left on the list of findings but are not eligible for the score points deduction.

The latest commit with the fixes reviewed by the auditors is indicated in the “Scope of Work” section of the report.

The Vidma team always provides a detailed description of the issues and recommendations on how to fix them.

Classification of found issues is graded according to 6 levels of severity described below:

Critical
The issue affects the contract in such a way that funds may be lost or allocated incorrectly, or the issue could result in a significant loss.
Example: Underflow/overflow, precisions, locked funds.
High
The issue significantly affects the ability of the contract to compile or operate. These are potential security or operational issues.
Example: Compilation errors, pausing/unpausing of some functionality, a random value, recursion, the logic that can use all gas from block (too many iterations in the loop), no limitations for locking period, cooldown, arithmetic errors which can cause underflow, etc.
Medium
The issue slightly impacts the contract’s ability to operate by slightly hindering its intended behavior.
Example: Absence of emergency withdrawal of funds, using assert for parameter sanitization.
Low
The issue doesn’t contain operational or security risks, but are more related to optimization of the codebase.
Example: Unused variables, inappropriate function visibility (public instead of external), useless importing of SCs, misuse or disuse of constant and immutable, absent indexing of parameters in events, absent events to track important state changes, absence of getters for important variables, usage of string as a key instead of a hash, etc.
Informational
Are classified as every point that increases onboarding time and code reading, as well as the issues which have no impact on the contract’s ability to operate.
Example: Code style, NatSpec, typos, license, refactoring, naming convention (or unclear naming), layout order, functions order, lack of any type of documentation.

Manual Report

Migration functionality possible pausing

High  MH – 01 |  Not relevant

The Migrator contract was not provided to the audit. If an incorrect migrator address will be passed to the Masterchef contract the migrate() function may fail.

In case, when more reward or staked tokens will be withdrawn with safeApprove() function user calls (withdraw(), leaveStaking(), emergencyWithdraw() functions) could be failed if token balance of the contract will be insufficient.

Recommendation:

Make sure that Migrator contract will proceed correctly.

Re-Audit:

Not relevant after removing functionality.

Access to withdraw all tokens from Masterchef SC

High  MH – 02 |  Resolved

The safeApprove() can approve a spender to withdraw all tokens by owner (including EG and LP tokens from the contract).

Recommendation:

Make sure that transferring more tokens after safeApprove() will not break operating of the contract.

Access to withdraw tokens of EGSwapPair by feeToSetter address

High  MH – 03 |  Resolved

In the EGSwapPair Smart Contract in the initialize() function there is two lines:

IERC20(token0).approve(IEGSwapFactory(factory).feeToSetter(), uint256(-1));
IERC20(token1).approve(IEGSwapFactory(factory).feeToSetter(), uint256(-1));

that makes possibility to withdraw pair tokens of users from the contract by feeToSetter. It can decrease user trust to the project.

Recommendation:

Provide the technical description of needing these lines or remove it.

Inappropriate function visibility

Low ML – 01 |  Resolved

In the audited codebase there are some functions that not called in the contract itself so their visibility can be improved from public to external:

  • function updateMultiplier();
  • function setEmergencyFee();
  • function add();
  • function set();
  • function deposit();
  • function withdraw();
  • function enterStaking();
  • function leaveStaking();
  • function emergencyWithdraw();
  • function safeApprove();
  • function dev();

Recommendation:

Consider changing visibility from public to external to safe gas usage on calling these functions.

Re-Audit:

Resolved but missed for emergencyWithdraw() function.

Re-Audit 2:

Resolved.

Useless IF statement

Low ML – 02 |  Resolved

There is a duplicated if statement in the withdraw() function of the Masterchef contract (L1208).

Recommendation:

Remove duplicated if (pending > 0):

if (pending > 0) {
            if (pending > 0) {
		             ...
            }
        }

msg.sender usage

Low ML – 03 |  Resolved

_msgSender() function can be used instead of msg.sender in Masterchef Smart Contract because it is inherited from Context Smart Contract. The _msgSender() function call requires less gas than msg.sender.

Recommendation:

Consider changing msg.sender to _msgSender().

Re-Audit:

Not resolved for calls in the emergencyWithdraw() function.

Re-Audit 2:

Resolved.

Optimization by ternary operator

Low ML – 04 |  Not relevant

In the Materchef Smart Contract there is deposit() functions that can be optimized to process result of ternary operator to transfer EG that will decrease using of if-else branching:

if (pending > 0) {
                if (validateStakingTime(msg.sender)) {
                    safeEGTransfer(msg.sender, pending, user.referral);
                } else {
                    safeEGTransfer(
                        msg.sender,
                        pending.mul(95).div(100),
                        user.referral
                    );
                }
            }

to:

if (pending > 0)
                safeEGTransfer(
                    msg.sender,
                    validateStakingTime(msg.sender)
                        ? pending
                        : pending.mul(95).div(100),
                    user.referral
                );

Recommendation:

Consider using ternary operators. It can also be used in the withdraw(), enterStaking(), leaveStaking() functions and validateStakingTime():

function validateStakingTime(address _staker) public view returns (bool) {
        UserInfo memory user = userInfo[0][_staker];
        if ((user.stakingTime + 30 days) < block.timestamp) {
            return true;
        }
        return false;
    }

to:

function validateStakingTime(address _staker) public view returns (bool) {
        return (userInfo[0][_staker].stakingTime + 30 days) < block.timestamp;
    }

Re-Audit:

Not relevant after logic changing.

Unnecessary default initialization

Low ML – 05 |  Resolved

There are state variables in Masterchef contract that are initialized by default type value that can be avoided:

  • uint256 public totalAllocPoint = 0;
  • uint256 public totalStakeAmount = 0; (or could be initialized with 1000 to miss initializing in the constructor);
  • uint256 points = 0; (in the updateStakingPool() function);
  • uint256 pid = 0; (in the massUpdatePools() function).

Recommendation:

Consider removing initialization by the default type value in the described cases.

Lack of delete operator

Low ML – 06 |  Resolved

In function emergencyWithdraw() assigns zero value to few struct fields. It is more efficient to use delete operator here to save gas usage on the function execution:

  • delete user.amount; (line 1303);
  • delete user.rewardDebt; (line 1304).

Recommendation:

Consider using delete operator to “clear” variables.

Lack of zero value check

Low ML – 07 |  Resolved

In function setMigrator() the migrator address is not checked if the passed address isn’t zero address. Same for the dev() function – if dev will be zero address then all transferred fees will be burned.

Recommendation:

Consider adding a requirement statement to check if the value isn’t zero value.

Unnecessary pairFor() internal call

Low ML – 08 |  Resolved

There is pairFor() call in the getReserves() function of EGSwapRouter contract (line 303) that returns a CREATE2 address for a pair but it’s not stored to the variable and duplicates the call in the next line.

Recommendation:

Consider removing the pairFor() call in the getReserves() function (line 303).

Simplifying the comparison for the requirement

Low ML – 09 |  Resolved

There is the requirement in the add() function of Masterchef contract (line 1051):

require(poolExist[address(_lpToken)] != true, "Existing pool");

that could be simplified to decrease gas usage:

require(!poolExist[address(_lpToken)], "Existing pool");

Recommendation:

Consider changing requirement for gas optimization.

Using of safe functions from SafeMath and SafeBEP20

Low TL – 01 |  Resolved

There are missed few points of using safe functions in Masterchef contract:

totalStakeAmount += _amount;

to:

totalStakeAmount = totalStakeAmount.add(_amount);

In safeEGTransfer() function:

if (_amount > egBal) {
            eg.transfer(_to, egBal);
        } else {
            eg.transfer(_to, _amount);
        }

to:

if (_amount > egBal) {
            eg.safeTransfer(_to, egBal);
        } else {
            eg.safeTransfer(_to, _amount);
        }

In emergencyWithdraw() function:

totalStakeAmount -= amount;

to:

totalStakeAmount = totalStakeAmount.sub(amount);

In emergencyWithdraw() function:

amount -= amount.mul(emergencyFee).div(100);

to:

amount = amount.sub(amount.mul(emergencyFee).div(100));

In leaveStaking() function:

totalStakeAmount -= _amount;

to:

totalStakeAmount = totalStakeAmount.sub(_amount);

In leaveStaking() function:

_amount -= unlockingFee;

to:

_amount = _amount.sub(unlockingFee);

Recommendation:

Use safe functions for calculating values and transferring tokens in all cases.

Lack of NatSpec comments

Informational  MI – 01 |  Unresolved

EGSwapFactory, EGSwapRouter, Masterchef and dependencies contracts are not covered by NatSpec. There are some functions that are covered only by simple comments. Follow:

Recommendation:

Consider covering all functions with qualitative and fully NatSpec comments.

Re-Audit:

Not resolved, only some comments were added. Check the examples with links added in the issue, use triple-slash and tags (@author, @notice, @param etc) to write NatSpec documentation.

Typo in the codebase

Informational  MI – 02 |  Resolved

There are some typos in the contracts code:

There is typo in comment, appears in Masterchef contract: poitns -> points.

Recommendation:

Consider fixing typos.

Wrong variable naming convention

Informational  MI – 03 |  Resolved

According to Solidity Style Guide, state and local variables should be in mixedCase style. In the Masterchef contract there are such variables that could be changed:

  • devaddr -> devAddr;
  • BONUS_MULTIPLIER -> bonusMultiplier (only constants can be written with uppercase).

Recommendation:

Consider following the Solidity style guide 0.6.12 and naming conventions.

Test Results

To verify the security of  contracts and the performance, a number of integration tests were carried out using the Hardhat testing framework.

In this section, we provide both tests written by EGSwap and tests written by Vidma auditors.

Vidma Coverage – 100%

Industry Standard – 95%

It is important to note that Vidma auditors do not modify, edit or add tests to the existing tests provided in the EGSwap repository. We write totally separate tests with code coverage of a minimum of 95% to meet the industry standards.

Tests written by Vidma auditors

Test Coverage

File
EGSwapFactory.sol
EGSwapRouter.sol
Masterchef.sol
All Files
File % Stmts % Branch % Funcs % Lines
EGSwapFactory.sol 100.00 100.00 100.00 100.00
EGSwapRouter.sol 100.00 100.00 100.00 100.00
Masterchef.sol 100.00 100.00 100.00 100.00
All Files 100.00 100.00 100.00 100.00

Test Results

  • EGSwapRouter{01,02}
    • EGSwapRouter01
      • factory, WETH(129ms)
      • addLiquidity(1405ms)
      • addLiquidityETH(1087ms)
      • removeLiquidity(2416ms)
      • removeLiquidityETH(3081ms)
      • removeLiquidityWithPermit(2093ms)
      • removeLiquidityETHWithPermit(2392ms)

Contract: YearlyVesting

  • initialization
    • Should set token address in the vesting contract correct
    • Should not set token address as zero address while deploy
  • lock
    • Should lock tokens to the vesting contract correct(57ms)
    • Should not lock tokens to the vesting contract if the user is zero address
    • Should not lock tokens to the vesting contract for the same user twice(63ms)
    • Should not lock tokens to the vesting contract if the caller is not the owner
  • withdraw
    • Should not withdraw any tokens from the vesting before 1 year passed(76ms)
    • Should withdraw tokens from the vesting contract after 1 year correct(119ms)
    • Should withdraw tokens from the vesting contract after 2 years correct(137ms)
    • Should withdraw tokens from the vesting contract after 2.5 years correct(210ms)
    • Should withdraw tokens from the vesting contract after 3 years correct(180ms)
    • Should withdraw tokens from the vesting contract after 3.5 years correct(210ms)
    • Should withdraw tokens from the vesting contract after 4 years correct(205ms)
    • Should not withdraw tokens if everything is claimed already(218ms)
    • Should not withdraw tokens if the user is not the receiver
  • _msg.data
    • Should return the data of the transaction correct
86 passing(29s)

We are delighted to have a chance to work with the Naplozz team and contribute to your company's success by reviewing and certifying the security of your smart contracts.

The statements made in this document should be interpreted neither as investment or legal advice, nor should its authors be held accountable for decisions made based on this document.

Vidma is a security audit company helping crypto companies ensure their code and products operate safely and as intended, enabling founders to sleep soundly at night. We specialize in auditing DeFi protocols, layer one protocols, and marketplace solutions. Our team consists of experienced and internationally trained specialists. Our company is based in Ukraine, known for its strong engineering, cryptography, and cybersecurity culture.
4