MonoX

One of MonoX's main objectives is to allow users for listing ERC20 tokens without the need for providing liquidity. Users can set arbitrary prices for tokens they list because the Monoswap.sol contract does not verify them against third-party data sources. The price of a given token can be updated only if it has not been swapped for at least 6000 blocks since the last exchange. In consequence, since the contract does not enforce minimum or maximum transaction amount, a malicious user can list tokens, price them way above market rate and keep the price on that level by doing microexchanges once every 6000 blocks thus effectively DoSing the pool.

Code Location

Monoswap.sol

function listNewToken (address _token, uint112 _price, 
    uint256 vusdAmount, 
    uint256 tokenAmount,
    address to) public returns(uint _pid, uint256 liquidity) {
    _pid = _createPool(_token, _price, PoolStatus.LISTED);
    liquidity = addLiquidityPair(_token, vusdAmount, tokenAmount, to);
}

Monoswap.sol

function _createPool (address _token, uint112 _price, PoolStatus _status) lock internal returns(uint256 _pid)  {
    require(tokenPoolStatus[_token]==0, "Monoswap: Token Exists");
    require (_token != address(vUSD), "Monoswap: vUSD pool not allowed");
    _pid = poolSize;
    pools[_token] = PoolInfo({
      token: _token,
      pid: _pid,
      vusdCredit: 0,
      vusdDebt: 0,
      tokenBalance: 0,
      lastPoolValue: 0,
      status: _status,
      price: _price
    });

    poolSize = _pid.add(1);
    tokenPoolStatus[_token]=1;

    // initialze pool's lasttradingblocknumber as the block number on which the pool is created
    lastTradedBlock[_token] = block.number;
}

Monoswap.sol

function swapExactTokenForToken(
    address tokenIn,
    address tokenOut,
    uint amountIn,
    uint amountOutMin,
    address to,
    uint deadline
  ) external virtual ensure(deadline) returns (uint amountOut) {
    amountOut = swapIn(tokenIn, tokenOut, msg.sender, to, amountIn);
    require(amountOut >= amountOutMin, 'Monoswap: INSUFFICIENT_OUTPUT_AMOUNT');
}

Monoswap.sol

    // record last trade's block number in mapping: lastTradedBlock
    lastTradedBlock[_token] = block.number;

Monoswap.sol

function updatePoolPrice(address _token, uint112 _newPrice) public onlyOwner {
    require(_newPrice > 0, 'Monoswap: zeroPriceNotAccept');
    require(tokenPoolStatus[_token] != 0, "Monoswap: PoolNotExist");

    PoolInfo storage pool = pools[_token];
    require(pool.price != _newPrice, "Monoswap: SamePriceNotAccept");

    require(block.number > lastTradedBlock[_token].add(6000), "Monoswap: PoolPriceUpdateLocked");
    pool.price = _newPrice;
    lastTradedBlock[_token] = block.number;
}

SOLVED: Fixed in commit #635a4cee2f2e50d854e06cac47c48aa0fafde2b0. Contract owner can now pause pools and temporarily disable swapping so that users with the PriceAdjuster role (assigned by the contract owner) can update prices.

In smart contracts, implementing a correct Access Control policy is an essential step to maintain security and decentralization for permissions on a token. All the features of the smart contract , such as mint/burn tokens and pause contracts are given by Access Control. For instance, Ownership is the most common form of Access Control. In other words, the owner of a contract (the account that deployed it by default) can do some administrative tasks on it. Nevertheless, other authorization levels are required to follow the principle of least privilege, also known as least authority. Briefly, any process, user or program only can access to the necessary resources or information. Otherwise, the ownership role is useful in a simple system, but more complex projects require the use of more roles by using Role-based access control.

Code Location

Monoswap.sol

function setFeeTo (address _feeTo) onlyOwner external {
    feeTo = _feeTo;
}

function setFees (uint16 _fees) onlyOwner external {
    require(_fees<1e3, "fees too large");
    fees = _fees;
}

function setDevFee (uint16 _devFee) onlyOwner external {
    require(_devFee<1e3, "devFee too large");
    devFee = _devFee;
}

// update status of a pool. onlyOwner.
function updatePoolStatus(address _token, PoolStatus _status) public onlyOwner {
    PoolInfo storage pool = pools[_token];
    pool.status = _status;
}

/**
 @dev update pools price if there were no active trading for the last 6000 blocks
 @notice Only owner callable, new price can neither be 0 nor be equal to old one
 @param _token pool identifider (token address)
 @param _newPrice new price in wei (uint112)
*/
function updatePoolPrice(address _token, uint112 _newPrice) public onlyOwner {
    require(_newPrice > 0, 'Monoswap: zeroPriceNotAccept');

SOLVED: Fixed in commit #635a4cee2f2e50d854e06cac47c48aa0fafde2b0. Several new roles were introduced.

An overflow happens when an arithmetic operation reaches the maximum size of a type. For instance, in Monoswap.sol, the getAmountOut method is subtracting fees from a fixed number and may end up overflowing the integer since the resulting value is not checked to be greater or equal 0. In computer programming, an integer overflow occurs when an arithmetic operation attempts to create a numeric value that is outside of the range that can be represented with a given number of bits – either larger than the maximum or lower than the minimum representable value.

Code Location

Monoswap.sol

function getAmountOut(address tokenIn, address tokenOut, 
    uint256 amountIn) public view returns (uint256 tokenInPrice, uint256 tokenOutPrice, 
    uint256 amountOut, uint256 tradeVusdValue) {
    require(amountIn > 0, 'Monoswap: INSUFFICIENT_INPUT_AMOUNT');

    uint256 amountInWithFee = amountIn.mul(1e5-fees)/1e5;
    address vusdAddress = address(vUSD);

Monoswap.sol

function getAmountIn(address tokenIn, address tokenOut, 
    uint256 amountOut) public view returns (uint256 tokenInPrice, uint256 tokenOutPrice, 
    uint256 amountIn, uint256 tradeVusdValue) {
    require(amountOut > 0, 'Monoswap: INSUFFICIENT_INPUT_AMOUNT');

    uint256 amountOutWithFee = amountOut.mul(1e5+fees)/1e5;
    address vusdAddress = address(vUSD);

SOLVED: MonoX is certain the integers reported will not overflow since the fees variable cannot be assigned value greater than 1e3.

Calls inside a loop might lead to a denial-of-service attack. In on of the functions discovered there is a for loop on variable pid that iterates up to the poolInfo array length. If this integer is evaluated at extremely large numbers this can cause a DoS.

Code Location

MonoswapStaking.sol

function massUpdatePools() public {
    uint256 length = poolInfo.length;
    for (uint256 pid = 0; pid < length; ++pid) {
        PoolInfo storage pool = poolInfo[pid];
        if (pool.bActive)
            updatePool(pid);
    }
}

SOLVED: MonoX is certain the DoS scenario is highly unlikely here since all external calls in this loop are made to MonoX-controlled contracts.

Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision. In this audit, there are multiple instances found where division is being performed before multiplication operation in contract file.

Code Location

MonoswapStaking.sol

if (user.oldReward > 0) {
    monoReward = monoReward.add(user.oldReward.mul(stakedAmount).div(user.amount).mul(1e12));
}

SOLVED: fixed in commit #ac21bee3f7f1d7df3529907b0afb0470b0236d07

Address validation is missing in multiple functions in contracts Monoswap.sol and MonoXPool.sol. This may result with users irreversibly locking their tokens when incorrect address is provided.

Code Location

Monoswap.sol

function mint (address account, uint256 id, uint256 amount) internal {
    monoXPool.mint(account, id, amount);
}

function burn (address account, uint256 id, uint256 amount) internal {
    monoXPool.burn(account, id, amount);
}

MonoXPool.sol

constructor (address _WETH) {
      WETH = _WETH;
}

MonoXPool.sol

function mint (address account, uint256 id, uint256 amount) public onlyOwner {
    totalSupply[id]=totalSupply[id].add(amount);
    _mint(account, id, amount, "");
}

function burn (address account, uint256 id, uint256 amount) public onlyOwner {
    totalSupply[id]=totalSupply[id].sub(amount);
    _burn(account, id, amount);
}

PARTIALLY SOLVED: Vulnerable function calls in Monoswap.sol have been removed but address validation is missing in MonoXPool.sol.

block.timestamp can be influenced by miners to a certain degree, so the testers should be warned that this may have some risk if miners collude on time manipulation to influence the price oracles.

Code Location

Monoswap.sol

modifier ensure(uint deadline) {
    require(deadline >= block.timestamp, 'Monoswap: EXPIRED');
    _;
}  

SOLVED: MonoX does not require timestamps to be extremely precise here (timescales are greater than 900 seconds)

In contract Monoswap.sol, tautology expressions have been detected. Such expressions are of no use since they always evaluate true/false regardless of the context they are used in.

Code Location

Monoswap.sol

if(_poolStatus == PoolStatus.LISTED){

      require (_vusdCredit>=0 && _vusdDebt==0, "Monoswap: unofficial pool cannot bear debt");
}

SOLVED: Tautology Expression was removed in commit #635a4cee2f2e50d854e06cac47c48aa0fafde2b0.

In public functions, array arguments are immediately copied to memory, while external functions can read directly from calldata. Reading calldata is cheaper than memory allocation. Public functions need to write the arguments to memory because public functions may be called internally. Internal calls are passed internally by pointers to memory. Thus, the function expects its arguments being located in memory when the compiler generates the code for an internal function. Also, methods do not necessarily have to be public if they are only called within the contract-in such case they should be marked internal.

Code Location

Monoswap.sol

function updatePoolStatus(address _token, PoolStatus _status) public onlyOwner {
    PoolInfo storage pool = pools[_token];
    pool.status = _status;
}

Monoswap.sol

function updatePoolPrice(address _token, uint112 _newPrice) public onlyOwner {
    require(_newPrice > 0, 'Monoswap: zeroPriceNotAccept');
    require(tokenPoolStatus[_token] != 0, "Monoswap: PoolNotExist");

    PoolInfo storage pool = pools[_token];

Monoswap.sol

function listNewToken (address _token, uint112 _price, 
    uint256 vusdAmount, 
    uint256 tokenAmount,
    address to) public returns(uint _pid, uint256 liquidity) {
    _pid = _createPool(_token, _price, PoolStatus.LISTED);
    liquidity = addLiquidityPair(_token, vusdAmount, tokenAmount, to);
  }
}

MonoswapStaking.sol

function set(
    uint256 _pid,
    uint256 _allocPoint,
    bool _withUpdate
) public onlyOwner {
    if (_withUpdate) {
        massUpdatePools();
    }

MonoswapStaking.sol

function stopPool(uint256 _pid) public onlyOwner {
    updatePool(_pid);

MonoswapStaking.sol

function migratePool(uint256 _oldPid, uint256 _newPid) public {
    PoolInfo storage oldPool = poolInfo[_oldPid];
    PoolInfo storage newPool = poolInfo[_newPid];

MonoswapStaking.sol

function deposit(uint256 _pid, uint256 _amount) public {
    PoolInfo storage pool = poolInfo[_pid];
    UserInfo storage user = userInfo[_pid][msg.sender];

MonoswapStaking.sol

function withdraw(uint256 _pid, uint256 _amount) public {
    PoolInfo storage pool = poolInfo[_pid];
    UserInfo storage user = userInfo[_pid][msg.sender];

MonoswapStaking.sol

function emergencyWithdraw(uint256 _pid) public {
    PoolInfo storage pool = poolInfo[_pid];
    UserInfo storage user = userInfo[_pid][msg.sender];

During the audit, it has been observed that integers with scientific notations are directly compared with function arguments.

Code Location

Monoswap.sol

function setFees (uint16 _fees) onlyOwner external {
    require(_fees<1e3, "fees too large");
    fees = _fees;
}

function setDevFee (uint16 _devFee) onlyOwner external {
    require(_devFee<1e3, "devFee too large");
    devFee = _devFee;
}

Monoswap.sol

function _mintFee (uint256 pid, uint256 lastPoolValue, uint256 newPoolValue) internal {

    uint256 _totalSupply = monoXPool.totalSupplyOf(pid);
    if(newPoolValue>lastPoolValue && lastPoolValue>0) {
      // safe ops, since newPoolValue>lastPoolValue
      uint256 deltaPoolValue = newPoolValue - lastPoolValue; 

      // safe ops, since newPoolValue = deltaPoolValue + lastPoolValue > deltaPoolValue
      uint256 devLiquidity = _totalSupply.mul(deltaPoolValue).mul(devFee).div(newPoolValue-deltaPoolValue)/1e5;
      monoXPool.mint(feeTo, pid, devLiquidity);
    }

}

Also lines #584 and #638 in Monoswap.sol.

Monoswap.sol

function getPool (address _token) view public returns (uint256 poolValue, 
    uint256 tokenBalanceVusdValue, uint256 vusdCredit, uint256 vusdDebt) {
    PoolInfo memory pool = pools[_token];
    vusdCredit = pool.vusdCredit;
    vusdDebt = pool.vusdDebt;
    tokenBalanceVusdValue = uint(pool.price).mul(pool.tokenBalance)/1e18;

    poolValue = tokenBalanceVusdValue.add(vusdCredit).sub(vusdDebt);
}

Also lines #258, #297, #322, #326, #569, #570, #590, #600, #605, #614, #628, #645, #656, #661, #671, #684, #715 and #767 in Monoswap.sol.

MonoswapStaking.sol

function initialize(
        MonoToken _mono,
        uint256 _monoPerPeriod,
        uint256 _blockPerPeriod,
        uint256 _decay
    ) public initializer {
        OwnableUpgradeable.__Ownable_init();
        __ERC1155Holder_init();
        mono = _mono;
        monoPerPeriod = _monoPerPeriod;
        blockPerPeriod = _blockPerPeriod;
        decay = _decay;
        startBlock = block.number;
        currentPeriod = 0;
        ratios[currentPeriod] = 1e12;
        totalAllocPoint = 0;
}

Also lines #176, #185, #211, #221, #232, #265, #297, #299, #316, #317, #336, #337, #359, #377, #378 and #389 in MonoswapStaking.sol.

ACKNOWLEDGED: MonoX refrain from introducing extra variables as it increases the contract size quite a bit and increase gas usage as well. Therefore they are trying not to have a variable unless it's necessary.